Hi Jiaxun, On 1/12/21 4:32 AM, Jiaxun Yang wrote: > Loongson IPI controller is a MMIO based simple level triggered > interrupt controller. It will trigger IRQ to it's upstream > processor when set register is written. > > It also has 8 32bit mailboxes to pass boot information to > secondary processor.
Hmm the datasheet describe them as 4 64bit mailboxes (also accessible in 32bit). I have no problem modelling this device as 8x32 to simplify, but the documentation should be accurate. Along with a comment describing NUM_MBOX value. > Signed-off-by: Jiaxun Yang <jiaxun.y...@flygoat.com> > --- > hw/intc/Kconfig | 3 + > hw/intc/loongson_ipi.c | 146 +++++++++++++++++++++++++++++++++ > hw/intc/meson.build | 1 + > include/hw/intc/loongson_ipi.h | 20 +++++ > 4 files changed, 170 insertions(+) > create mode 100644 hw/intc/loongson_ipi.c > create mode 100644 include/hw/intc/loongson_ipi.h > > diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig > index c18d11142a..0e15102662 100644 > --- a/hw/intc/Kconfig > +++ b/hw/intc/Kconfig > @@ -59,6 +59,9 @@ config RX_ICU > config LOONGSON_LIOINTC > bool > > +config LOONGSON_IPI > + bool > + > config SIFIVE_CLINT > bool > > diff --git a/hw/intc/loongson_ipi.c b/hw/intc/loongson_ipi.c > new file mode 100644 > index 0000000000..7246f05f9e > --- /dev/null > +++ b/hw/intc/loongson_ipi.c > @@ -0,0 +1,146 @@ > +/* > + * QEMU Loongson Inter Processor Interrupt Controller > + * > + * Copyright (c) 2020-2021 Jiaxun Yang <jiaxun.y...@flygoat.com> > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <https://www.gnu.org/licenses/>. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "hw/sysbus.h" > +#include "qemu/module.h" > +#include "qemu/log.h" > +#include "hw/irq.h" > +#include "hw/qdev-properties.h" > +#include "hw/intc/loongson_ipi.h" > + > +#define R_ISR 0 > +#define R_IEN 1 > +#define R_SET 2 > +#define R_CLR 3 > +/* No register between 0x10~0x20 */ > +#define R_MBOX0 8 > +#define NUM_MBOX 8 > +#define R_END 16 Can you use an enum please? > + > +struct loongson_ipi { > + SysBusDevice parent_obj; > + > + MemoryRegion mmio; > + qemu_irq parent_irq; > + > + uint32_t isr; > + uint32_t ien; > + uint32_t mbox[NUM_MBOX]; > +}; > + > +static uint64_t > +ipi_read(void *opaque, hwaddr addr, unsigned int size) > +{ > + struct loongson_ipi *p = opaque; > + uint64_t r = 0; > + > + addr >>= 2; > + switch (addr) { > + case R_ISR: > + r = p->isr; > + break; > + case R_IEN: > + r = p->ien; > + break; > + case R_MBOX0 ... (R_END - 1): > + r = p->mbox[addr - R_MBOX0]; > + break; Logging here write-only registers (R_SET, R_CLR) as LOG_GUEST_ERROR might result useful. > + default: > + break; > + } > + > + qemu_log_mask(CPU_LOG_INT, > + "%s: size=%d, addr=%"HWADDR_PRIx", val=%"PRIx64"\n", > + __func__, size, addr, r); CPU_LOG_INT is used for the architectural part (within GS464V). For hardware we use trace events. Do you mind changing? > + > + return r; > +} > + > +static void > +ipi_write(void *opaque, hwaddr addr, > + uint64_t val64, unsigned int size) > +{ > + struct loongson_ipi *p = opaque; > + uint32_t value = val64; > + > + addr >>= 2; > + switch (addr) { > + case R_ISR: > + /* Do nothing */ > + break; > + case R_IEN: > + p->ien = value; > + break; > + case R_SET: > + p->isr |= value; > + break; > + case R_CLR: > + p->isr &= ~value; > + break; > + case R_MBOX0 ... (R_END - 1): > + p->mbox[addr - R_MBOX0] = value; > + break; > + default: > + break; > + } > + p->isr &= p->ien; > + > + qemu_log_mask(CPU_LOG_INT, > + "%s: size=%d, addr=%"HWADDR_PRIx", val=%"PRIx32"\n", > + __func__, size, addr, value); Ditto. > + > + qemu_set_irq(p->parent_irq, !!p->isr); > +} > + > +static const MemoryRegionOps pic_ops = { > + .read = ipi_read, > + .write = ipi_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > + .valid = { > + .min_access_size = 4, > + .max_access_size = 4 > + } 64-bit access are valid. You probably want: .impl = { .min_access_size = 4, .max_access_size = 4 }, .valid = { .min_access_size = 4, .max_access_size = 8 }, > +}; > + > +static void loongson_ipi_init(Object *obj) > +{ > + struct loongson_ipi *p = LOONGSON_IPI(obj); > + > + sysbus_init_irq(SYS_BUS_DEVICE(obj), &p->parent_irq); > + > + memory_region_init_io(&p->mmio, obj, &pic_ops, p, "loongson.ipi", > + R_END * 4); > + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &p->mmio); > +} > + You forgot the reset() method which clear the registers. > +static const TypeInfo loongson_ipi_info = { > + .name = TYPE_LOONGSON_IPI, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(struct loongson_ipi), > + .instance_init = loongson_ipi_init, > +}; > + > +static void loongson_ipi_register_types(void) > +{ > + type_register_static(&loongson_ipi_info); > +} > + > +type_init(loongson_ipi_register_types) > diff --git a/hw/intc/meson.build b/hw/intc/meson.build > index 53cba11569..5257c5fb94 100644 > --- a/hw/intc/meson.build > +++ b/hw/intc/meson.build > @@ -36,6 +36,7 @@ specific_ss.add(when: 'CONFIG_GRLIB', if_true: > files('grlib_irqmp.c')) > specific_ss.add(when: 'CONFIG_IBEX', if_true: files('ibex_plic.c')) > specific_ss.add(when: 'CONFIG_IOAPIC', if_true: files('ioapic.c')) > specific_ss.add(when: 'CONFIG_LOONGSON_LIOINTC', if_true: > files('loongson_liointc.c')) > +specific_ss.add(when: 'CONFIG_LOONGSON_IPI', if_true: > files('loongson_ipi.c')) > specific_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('mips_gic.c')) > specific_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_intc.c')) > specific_ss.add(when: 'CONFIG_OMPIC', if_true: files('ompic.c')) > diff --git a/include/hw/intc/loongson_ipi.h b/include/hw/intc/loongson_ipi.h > new file mode 100644 > index 0000000000..a535c467bf > --- /dev/null > +++ b/include/hw/intc/loongson_ipi.h If possible use the scripts/git.orderfile to ease review. Minor comments, but the patch is good! Thanks, Phil.