[PATCH v3 0/4] hw/mips/loongson3_virt: Implement IPI support

[PATCH v3 0/4] hw/mips/loongson3_virt: Implement IPI support

[Jiaxun Yang]

Hi all,

This series enabled IPI support for loongson3 virt board, loosely
based on my previous work[1].
It generalized loongarch_ipi device to share among both loongarch
and MIPS machines. 

Thanks

[1]: https://lore.kernel.org/all/20230521102307.87081-1-jiaxun.yang@flygoat.com/

To: qemu-devel@nongnu.org
Cc: Song Gao <gaosong@loongson.cn>

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
Changes in v3:
- Fix up sign off lines.
- Link to v2: https://lore.kernel.org/r/20240605-loongson3-ipi-v2-0-da26282e03a5@flygoat.com

Changes in v2:
- Add a patch to remove loongarch_ipi.c (Gao)
- Change iocsr alias owner to CPU (philmd)
- New patch to remove ipi_getcpu
- Link to v1: https://lore.kernel.org/r/20240508-loongson3-ipi-v1-0-1a7b67704664@flygoat.com

---
Jiaxun Yang (4):
      hw/intc: Remove loongarch_ipi.c
      hw/intc/loongson_ipi: Provide per core MMIO address spaces
      hw/mips/loongson3_virt: Wire up loongson_ipi device
      hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id

 hw/intc/loongarch_ipi.c        | 347 -----------------------------------------
 hw/intc/loongson_ipi.c         | 120 +++++++-------
 hw/mips/Kconfig                |   1 +
 hw/mips/loongson3_bootp.c      |   2 -
 hw/mips/loongson3_bootp.h      |   3 +
 hw/mips/loongson3_virt.c       |  39 ++++-
 include/hw/intc/loongson_ipi.h |   2 +
 7 files changed, 108 insertions(+), 406 deletions(-)
---
base-commit: d67a6e054b92e5e1cbb7b0bd5782a670cc7f0df7
change-id: 20240508-loongson3-ipi-65e99b47c400

Best regards,
-- 
Jiaxun Yang <jiaxun.yang@flygoat.com>

[PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Jiaxun Yang]

It was missed out in previous commit.

Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/intc/loongarch_ipi.c | 347 ------------------------------------------------
 1 file changed, 347 deletions(-)

diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
deleted file mode 100644
index 44b3b9c138d6..000000000000
--- a/hw/intc/loongarch_ipi.c
+++ /dev/null
@@ -1,347 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * LoongArch ipi interrupt support
- *
- * Copyright (C) 2021 Loongson Technology Corporation Limited
- */
-
-#include "qemu/osdep.h"
-#include "hw/boards.h"
-#include "hw/sysbus.h"
-#include "hw/intc/loongarch_ipi.h"
-#include "hw/irq.h"
-#include "hw/qdev-properties.h"
-#include "qapi/error.h"
-#include "qemu/log.h"
-#include "exec/address-spaces.h"
-#include "migration/vmstate.h"
-#include "target/loongarch/cpu.h"
-#include "trace.h"
-
-static MemTxResult loongarch_ipi_readl(void *opaque, hwaddr addr,
-                                       uint64_t *data,
-                                       unsigned size, MemTxAttrs attrs)
-{
-    IPICore *s;
-    LoongArchIPI *ipi = opaque;
-    uint64_t ret = 0;
-    int index = 0;
-
-    s = &ipi->cpu[attrs.requester_id];
-    addr &= 0xff;
-    switch (addr) {
-    case CORE_STATUS_OFF:
-        ret = s->status;
-        break;
-    case CORE_EN_OFF:
-        ret = s->en;
-        break;
-    case CORE_SET_OFF:
-        ret = 0;
-        break;
-    case CORE_CLEAR_OFF:
-        ret = 0;
-        break;
-    case CORE_BUF_20 ... CORE_BUF_38 + 4:
-        index = (addr - CORE_BUF_20) >> 2;
-        ret = s->buf[index];
-        break;
-    default:
-        qemu_log_mask(LOG_UNIMP, "invalid read: %x", (uint32_t)addr);
-        break;
-    }
-
-    trace_loongarch_ipi_read(size, (uint64_t)addr, ret);
-    *data = ret;
-    return MEMTX_OK;
-}
-
-static void send_ipi_data(CPULoongArchState *env, uint64_t val, hwaddr addr,
-                          MemTxAttrs attrs)
-{
-    int i, mask = 0, data = 0;
-
-    /*
-     * bit 27-30 is mask for byte writing,
-     * if the mask is 0, we need not to do anything.
-     */
-    if ((val >> 27) & 0xf) {
-        data = address_space_ldl(env->address_space_iocsr, addr,
-                                 attrs, NULL);
-        for (i = 0; i < 4; i++) {
-            /* get mask for byte writing */
-            if (val & (0x1 << (27 + i))) {
-                mask |= 0xff << (i * 8);
-            }
-        }
-    }
-
-    data &= mask;
-    data |= (val >> 32) & ~mask;
-    address_space_stl(env->address_space_iocsr, addr,
-                      data, attrs, NULL);
-}
-
-static int archid_cmp(const void *a, const void *b)
-{
-   CPUArchId *archid_a = (CPUArchId *)a;
-   CPUArchId *archid_b = (CPUArchId *)b;
-
-   return archid_a->arch_id - archid_b->arch_id;
-}
-
-static CPUArchId *find_cpu_by_archid(MachineState *ms, uint32_t id)
-{
-    CPUArchId apic_id, *found_cpu;
-
-    apic_id.arch_id = id;
-    found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
-        ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
-        archid_cmp);
-
-    return found_cpu;
-}
-
-static CPUState *ipi_getcpu(int arch_id)
-{
-    MachineState *machine = MACHINE(qdev_get_machine());
-    CPUArchId *archid;
-
-    archid = find_cpu_by_archid(machine, arch_id);
-    if (archid) {
-        return CPU(archid->cpu);
-    }
-
-    return NULL;
-}
-
-static MemTxResult mail_send(uint64_t val, MemTxAttrs attrs)
-{
-    uint32_t cpuid;
-    hwaddr addr;
-    CPUState *cs;
-
-    cpuid = extract32(val, 16, 10);
-    cs = ipi_getcpu(cpuid);
-    if (cs == NULL) {
-        return MEMTX_DECODE_ERROR;
-    }
-
-    /* override requester_id */
-    addr = SMP_IPI_MAILBOX + CORE_BUF_20 + (val & 0x1c);
-    attrs.requester_id = cs->cpu_index;
-    send_ipi_data(&LOONGARCH_CPU(cs)->env, val, addr, attrs);
-    return MEMTX_OK;
-}
-
-static MemTxResult any_send(uint64_t val, MemTxAttrs attrs)
-{
-    uint32_t cpuid;
-    hwaddr addr;
-    CPUState *cs;
-
-    cpuid = extract32(val, 16, 10);
-    cs = ipi_getcpu(cpuid);
-    if (cs == NULL) {
-        return MEMTX_DECODE_ERROR;
-    }
-
-    /* override requester_id */
-    addr = val & 0xffff;
-    attrs.requester_id = cs->cpu_index;
-    send_ipi_data(&LOONGARCH_CPU(cs)->env, val, addr, attrs);
-    return MEMTX_OK;
-}
-
-static MemTxResult loongarch_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
-                                        unsigned size, MemTxAttrs attrs)
-{
-    LoongArchIPI *ipi = opaque;
-    IPICore *s;
-    int index = 0;
-    uint32_t cpuid;
-    uint8_t vector;
-    CPUState *cs;
-
-    s = &ipi->cpu[attrs.requester_id];
-    addr &= 0xff;
-    trace_loongarch_ipi_write(size, (uint64_t)addr, val);
-    switch (addr) {
-    case CORE_STATUS_OFF:
-        qemu_log_mask(LOG_GUEST_ERROR, "can not be written");
-        break;
-    case CORE_EN_OFF:
-        s->en = val;
-        break;
-    case CORE_SET_OFF:
-        s->status |= val;
-        if (s->status != 0 && (s->status & s->en) != 0) {
-            qemu_irq_raise(s->irq);
-        }
-        break;
-    case CORE_CLEAR_OFF:
-        s->status &= ~val;
-        if (s->status == 0 && s->en != 0) {
-            qemu_irq_lower(s->irq);
-        }
-        break;
-    case CORE_BUF_20 ... CORE_BUF_38 + 4:
-        index = (addr - CORE_BUF_20) >> 2;
-        s->buf[index] = val;
-        break;
-    case IOCSR_IPI_SEND:
-        cpuid = extract32(val, 16, 10);
-        /* IPI status vector */
-        vector = extract8(val, 0, 5);
-        cs = ipi_getcpu(cpuid);
-        if (cs == NULL) {
-            return MEMTX_DECODE_ERROR;
-        }
-
-        /* override requester_id */
-        attrs.requester_id = cs->cpu_index;
-        loongarch_ipi_writel(ipi, CORE_SET_OFF, BIT(vector), 4, attrs);
-        break;
-    default:
-        qemu_log_mask(LOG_UNIMP, "invalid write: %x", (uint32_t)addr);
-        break;
-    }
-
-    return MEMTX_OK;
-}
-
-static const MemoryRegionOps loongarch_ipi_ops = {
-    .read_with_attrs = loongarch_ipi_readl,
-    .write_with_attrs = loongarch_ipi_writel,
-    .impl.min_access_size = 4,
-    .impl.max_access_size = 4,
-    .valid.min_access_size = 4,
-    .valid.max_access_size = 8,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-/* mail send and any send only support writeq */
-static MemTxResult loongarch_ipi_writeq(void *opaque, hwaddr addr, uint64_t val,
-                                        unsigned size, MemTxAttrs attrs)
-{
-    MemTxResult ret = MEMTX_OK;
-
-    addr &= 0xfff;
-    switch (addr) {
-    case MAIL_SEND_OFFSET:
-        ret = mail_send(val, attrs);
-        break;
-    case ANY_SEND_OFFSET:
-        ret = any_send(val, attrs);
-        break;
-    default:
-       break;
-    }
-
-    return ret;
-}
-
-static const MemoryRegionOps loongarch_ipi64_ops = {
-    .write_with_attrs = loongarch_ipi_writeq,
-    .impl.min_access_size = 8,
-    .impl.max_access_size = 8,
-    .valid.min_access_size = 8,
-    .valid.max_access_size = 8,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
-{
-    LoongArchIPI *s = LOONGARCH_IPI(dev);
-    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-    int i;
-
-    if (s->num_cpu == 0) {
-        error_setg(errp, "num-cpu must be at least 1");
-        return;
-    }
-
-    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), &loongarch_ipi_ops,
-                          s, "loongarch_ipi_iocsr", 0x48);
-
-    /* loongarch_ipi_iocsr performs re-entrant IO through ipi_send */
-    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
-
-    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
-
-    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
-                          &loongarch_ipi64_ops,
-                          s, "loongarch_ipi64_iocsr", 0x118);
-    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
-
-    s->cpu = g_new0(IPICore, s->num_cpu);
-    if (s->cpu == NULL) {
-        error_setg(errp, "Memory allocation for ExtIOICore faile");
-        return;
-    }
-
-    for (i = 0; i < s->num_cpu; i++) {
-        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
-    }
-}
-
-static const VMStateDescription vmstate_ipi_core = {
-    .name = "ipi-single",
-    .version_id = 2,
-    .minimum_version_id = 2,
-    .fields = (const VMStateField[]) {
-        VMSTATE_UINT32(status, IPICore),
-        VMSTATE_UINT32(en, IPICore),
-        VMSTATE_UINT32(set, IPICore),
-        VMSTATE_UINT32(clear, IPICore),
-        VMSTATE_UINT32_ARRAY(buf, IPICore, IPI_MBX_NUM * 2),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static const VMStateDescription vmstate_loongarch_ipi = {
-    .name = TYPE_LOONGARCH_IPI,
-    .version_id = 2,
-    .minimum_version_id = 2,
-    .fields = (const VMStateField[]) {
-        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, LoongArchIPI, num_cpu,
-                         vmstate_ipi_core, IPICore),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static Property ipi_properties[] = {
-    DEFINE_PROP_UINT32("num-cpu", LoongArchIPI, num_cpu, 1),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-
-    dc->realize = loongarch_ipi_realize;
-    device_class_set_props(dc, ipi_properties);
-    dc->vmsd = &vmstate_loongarch_ipi;
-}
-
-static void loongarch_ipi_finalize(Object *obj)
-{
-    LoongArchIPI *s = LOONGARCH_IPI(obj);
-
-    g_free(s->cpu);
-}
-
-static const TypeInfo loongarch_ipi_info = {
-    .name          = TYPE_LOONGARCH_IPI,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(LoongArchIPI),
-    .class_init    = loongarch_ipi_class_init,
-    .instance_finalize = loongarch_ipi_finalize,
-};
-
-static void loongarch_ipi_register_types(void)
-{
-    type_register_static(&loongarch_ipi_info);
-}
-
-type_init(loongarch_ipi_register_types)

-- 
2.43.0

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[gaosong]

在 2024/6/5 上午10:15, Jiaxun Yang 写道:
> It was missed out in previous commit.
>
> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
Reviewed-by: Song Gao <gaosong@loongson.cn>

Thanks.
Song Gao
>   hw/intc/loongarch_ipi.c | 347 ------------------------------------------------
>   1 file changed, 347 deletions(-)
>
> diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
> deleted file mode 100644
> index 44b3b9c138d6..000000000000
> --- a/hw/intc/loongarch_ipi.c
> +++ /dev/null
> @@ -1,347 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * LoongArch ipi interrupt support
> - *
> - * Copyright (C) 2021 Loongson Technology Corporation Limited
> - */
> -
> -#include "qemu/osdep.h"
> -#include "hw/boards.h"
> -#include "hw/sysbus.h"
> -#include "hw/intc/loongarch_ipi.h"
> -#include "hw/irq.h"
> -#include "hw/qdev-properties.h"
> -#include "qapi/error.h"
> -#include "qemu/log.h"
> -#include "exec/address-spaces.h"
> -#include "migration/vmstate.h"
> -#include "target/loongarch/cpu.h"
> -#include "trace.h"
> -
> -static MemTxResult loongarch_ipi_readl(void *opaque, hwaddr addr,
> -                                       uint64_t *data,
> -                                       unsigned size, MemTxAttrs attrs)
> -{
> -    IPICore *s;
> -    LoongArchIPI *ipi = opaque;
> -    uint64_t ret = 0;
> -    int index = 0;
> -
> -    s = &ipi->cpu[attrs.requester_id];
> -    addr &= 0xff;
> -    switch (addr) {
> -    case CORE_STATUS_OFF:
> -        ret = s->status;
> -        break;
> -    case CORE_EN_OFF:
> -        ret = s->en;
> -        break;
> -    case CORE_SET_OFF:
> -        ret = 0;
> -        break;
> -    case CORE_CLEAR_OFF:
> -        ret = 0;
> -        break;
> -    case CORE_BUF_20 ... CORE_BUF_38 + 4:
> -        index = (addr - CORE_BUF_20) >> 2;
> -        ret = s->buf[index];
> -        break;
> -    default:
> -        qemu_log_mask(LOG_UNIMP, "invalid read: %x", (uint32_t)addr);
> -        break;
> -    }
> -
> -    trace_loongarch_ipi_read(size, (uint64_t)addr, ret);
> -    *data = ret;
> -    return MEMTX_OK;
> -}
> -
> -static void send_ipi_data(CPULoongArchState *env, uint64_t val, hwaddr addr,
> -                          MemTxAttrs attrs)
> -{
> -    int i, mask = 0, data = 0;
> -
> -    /*
> -     * bit 27-30 is mask for byte writing,
> -     * if the mask is 0, we need not to do anything.
> -     */
> -    if ((val >> 27) & 0xf) {
> -        data = address_space_ldl(env->address_space_iocsr, addr,
> -                                 attrs, NULL);
> -        for (i = 0; i < 4; i++) {
> -            /* get mask for byte writing */
> -            if (val & (0x1 << (27 + i))) {
> -                mask |= 0xff << (i * 8);
> -            }
> -        }
> -    }
> -
> -    data &= mask;
> -    data |= (val >> 32) & ~mask;
> -    address_space_stl(env->address_space_iocsr, addr,
> -                      data, attrs, NULL);
> -}
> -
> -static int archid_cmp(const void *a, const void *b)
> -{
> -   CPUArchId *archid_a = (CPUArchId *)a;
> -   CPUArchId *archid_b = (CPUArchId *)b;
> -
> -   return archid_a->arch_id - archid_b->arch_id;
> -}
> -
> -static CPUArchId *find_cpu_by_archid(MachineState *ms, uint32_t id)
> -{
> -    CPUArchId apic_id, *found_cpu;
> -
> -    apic_id.arch_id = id;
> -    found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
> -        ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
> -        archid_cmp);
> -
> -    return found_cpu;
> -}
> -
> -static CPUState *ipi_getcpu(int arch_id)
> -{
> -    MachineState *machine = MACHINE(qdev_get_machine());
> -    CPUArchId *archid;
> -
> -    archid = find_cpu_by_archid(machine, arch_id);
> -    if (archid) {
> -        return CPU(archid->cpu);
> -    }
> -
> -    return NULL;
> -}
> -
> -static MemTxResult mail_send(uint64_t val, MemTxAttrs attrs)
> -{
> -    uint32_t cpuid;
> -    hwaddr addr;
> -    CPUState *cs;
> -
> -    cpuid = extract32(val, 16, 10);
> -    cs = ipi_getcpu(cpuid);
> -    if (cs == NULL) {
> -        return MEMTX_DECODE_ERROR;
> -    }
> -
> -    /* override requester_id */
> -    addr = SMP_IPI_MAILBOX + CORE_BUF_20 + (val & 0x1c);
> -    attrs.requester_id = cs->cpu_index;
> -    send_ipi_data(&LOONGARCH_CPU(cs)->env, val, addr, attrs);
> -    return MEMTX_OK;
> -}
> -
> -static MemTxResult any_send(uint64_t val, MemTxAttrs attrs)
> -{
> -    uint32_t cpuid;
> -    hwaddr addr;
> -    CPUState *cs;
> -
> -    cpuid = extract32(val, 16, 10);
> -    cs = ipi_getcpu(cpuid);
> -    if (cs == NULL) {
> -        return MEMTX_DECODE_ERROR;
> -    }
> -
> -    /* override requester_id */
> -    addr = val & 0xffff;
> -    attrs.requester_id = cs->cpu_index;
> -    send_ipi_data(&LOONGARCH_CPU(cs)->env, val, addr, attrs);
> -    return MEMTX_OK;
> -}
> -
> -static MemTxResult loongarch_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
> -                                        unsigned size, MemTxAttrs attrs)
> -{
> -    LoongArchIPI *ipi = opaque;
> -    IPICore *s;
> -    int index = 0;
> -    uint32_t cpuid;
> -    uint8_t vector;
> -    CPUState *cs;
> -
> -    s = &ipi->cpu[attrs.requester_id];
> -    addr &= 0xff;
> -    trace_loongarch_ipi_write(size, (uint64_t)addr, val);
> -    switch (addr) {
> -    case CORE_STATUS_OFF:
> -        qemu_log_mask(LOG_GUEST_ERROR, "can not be written");
> -        break;
> -    case CORE_EN_OFF:
> -        s->en = val;
> -        break;
> -    case CORE_SET_OFF:
> -        s->status |= val;
> -        if (s->status != 0 && (s->status & s->en) != 0) {
> -            qemu_irq_raise(s->irq);
> -        }
> -        break;
> -    case CORE_CLEAR_OFF:
> -        s->status &= ~val;
> -        if (s->status == 0 && s->en != 0) {
> -            qemu_irq_lower(s->irq);
> -        }
> -        break;
> -    case CORE_BUF_20 ... CORE_BUF_38 + 4:
> -        index = (addr - CORE_BUF_20) >> 2;
> -        s->buf[index] = val;
> -        break;
> -    case IOCSR_IPI_SEND:
> -        cpuid = extract32(val, 16, 10);
> -        /* IPI status vector */
> -        vector = extract8(val, 0, 5);
> -        cs = ipi_getcpu(cpuid);
> -        if (cs == NULL) {
> -            return MEMTX_DECODE_ERROR;
> -        }
> -
> -        /* override requester_id */
> -        attrs.requester_id = cs->cpu_index;
> -        loongarch_ipi_writel(ipi, CORE_SET_OFF, BIT(vector), 4, attrs);
> -        break;
> -    default:
> -        qemu_log_mask(LOG_UNIMP, "invalid write: %x", (uint32_t)addr);
> -        break;
> -    }
> -
> -    return MEMTX_OK;
> -}
> -
> -static const MemoryRegionOps loongarch_ipi_ops = {
> -    .read_with_attrs = loongarch_ipi_readl,
> -    .write_with_attrs = loongarch_ipi_writel,
> -    .impl.min_access_size = 4,
> -    .impl.max_access_size = 4,
> -    .valid.min_access_size = 4,
> -    .valid.max_access_size = 8,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> -
> -/* mail send and any send only support writeq */
> -static MemTxResult loongarch_ipi_writeq(void *opaque, hwaddr addr, uint64_t val,
> -                                        unsigned size, MemTxAttrs attrs)
> -{
> -    MemTxResult ret = MEMTX_OK;
> -
> -    addr &= 0xfff;
> -    switch (addr) {
> -    case MAIL_SEND_OFFSET:
> -        ret = mail_send(val, attrs);
> -        break;
> -    case ANY_SEND_OFFSET:
> -        ret = any_send(val, attrs);
> -        break;
> -    default:
> -       break;
> -    }
> -
> -    return ret;
> -}
> -
> -static const MemoryRegionOps loongarch_ipi64_ops = {
> -    .write_with_attrs = loongarch_ipi_writeq,
> -    .impl.min_access_size = 8,
> -    .impl.max_access_size = 8,
> -    .valid.min_access_size = 8,
> -    .valid.max_access_size = 8,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> -
> -static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
> -{
> -    LoongArchIPI *s = LOONGARCH_IPI(dev);
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> -    int i;
> -
> -    if (s->num_cpu == 0) {
> -        error_setg(errp, "num-cpu must be at least 1");
> -        return;
> -    }
> -
> -    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), &loongarch_ipi_ops,
> -                          s, "loongarch_ipi_iocsr", 0x48);
> -
> -    /* loongarch_ipi_iocsr performs re-entrant IO through ipi_send */
> -    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
> -
> -    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
> -
> -    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
> -                          &loongarch_ipi64_ops,
> -                          s, "loongarch_ipi64_iocsr", 0x118);
> -    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
> -
> -    s->cpu = g_new0(IPICore, s->num_cpu);
> -    if (s->cpu == NULL) {
> -        error_setg(errp, "Memory allocation for ExtIOICore faile");
> -        return;
> -    }
> -
> -    for (i = 0; i < s->num_cpu; i++) {
> -        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
> -    }
> -}
> -
> -static const VMStateDescription vmstate_ipi_core = {
> -    .name = "ipi-single",
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> -    .fields = (const VMStateField[]) {
> -        VMSTATE_UINT32(status, IPICore),
> -        VMSTATE_UINT32(en, IPICore),
> -        VMSTATE_UINT32(set, IPICore),
> -        VMSTATE_UINT32(clear, IPICore),
> -        VMSTATE_UINT32_ARRAY(buf, IPICore, IPI_MBX_NUM * 2),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
> -static const VMStateDescription vmstate_loongarch_ipi = {
> -    .name = TYPE_LOONGARCH_IPI,
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> -    .fields = (const VMStateField[]) {
> -        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, LoongArchIPI, num_cpu,
> -                         vmstate_ipi_core, IPICore),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
> -static Property ipi_properties[] = {
> -    DEFINE_PROP_UINT32("num-cpu", LoongArchIPI, num_cpu, 1),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -
> -    dc->realize = loongarch_ipi_realize;
> -    device_class_set_props(dc, ipi_properties);
> -    dc->vmsd = &vmstate_loongarch_ipi;
> -}
> -
> -static void loongarch_ipi_finalize(Object *obj)
> -{
> -    LoongArchIPI *s = LOONGARCH_IPI(obj);
> -
> -    g_free(s->cpu);
> -}
> -
> -static const TypeInfo loongarch_ipi_info = {
> -    .name          = TYPE_LOONGARCH_IPI,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(LoongArchIPI),
> -    .class_init    = loongarch_ipi_class_init,
> -    .instance_finalize = loongarch_ipi_finalize,
> -};
> -
> -static void loongarch_ipi_register_types(void)
> -{
> -    type_register_static(&loongarch_ipi_info);
> -}
> -
> -type_init(loongarch_ipi_register_types)
>

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Philippe Mathieu-Daudé]

On 5/6/24 04:15, Jiaxun Yang wrote:
> It was missed out in previous commit.
> 
> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   hw/intc/loongarch_ipi.c | 347 ------------------------------------------------
>   1 file changed, 347 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[maobibo]

On 2024/6/5 上午10:15, Jiaxun Yang wrote:
> It was missed out in previous commit.
> 
> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   hw/intc/loongarch_ipi.c | 347 ------------------------------------------------
>   1 file changed, 347 deletions(-)
> 
> diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
> deleted file mode 100644
> index 44b3b9c138d6..000000000000
> --- a/hw/intc/loongarch_ipi.c
> +++ /dev/null
> @@ -1,347 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * LoongArch ipi interrupt support
> - *
> - * Copyright (C) 2021 Loongson Technology Corporation Limited
> - */
> -
> -#include "qemu/osdep.h"
> -#include "hw/boards.h"
> -#include "hw/sysbus.h"
> -#include "hw/intc/loongarch_ipi.h"
> -#include "hw/irq.h"
> -#include "hw/qdev-properties.h"
> -#include "qapi/error.h"
> -#include "qemu/log.h"
> -#include "exec/address-spaces.h"
> -#include "migration/vmstate.h"
> -#include "target/loongarch/cpu.h"
> -#include "trace.h"
> -
> -static MemTxResult loongarch_ipi_readl(void *opaque, hwaddr addr,
> -                                       uint64_t *data,
> -                                       unsigned size, MemTxAttrs attrs)
> -{
> -    IPICore *s;
> -    LoongArchIPI *ipi = opaque;
> -    uint64_t ret = 0;
> -    int index = 0;
> -
> -    s = &ipi->cpu[attrs.requester_id];
> -    addr &= 0xff;
> -    switch (addr) {
> -    case CORE_STATUS_OFF:
> -        ret = s->status;
> -        break;
> -    case CORE_EN_OFF:
> -        ret = s->en;
> -        break;
> -    case CORE_SET_OFF:
> -        ret = 0;
> -        break;
> -    case CORE_CLEAR_OFF:
> -        ret = 0;
> -        break;
> -    case CORE_BUF_20 ... CORE_BUF_38 + 4:
> -        index = (addr - CORE_BUF_20) >> 2;
> -        ret = s->buf[index];
> -        break;
> -    default:
> -        qemu_log_mask(LOG_UNIMP, "invalid read: %x", (uint32_t)addr);
> -        break;
> -    }
> -
> -    trace_loongarch_ipi_read(size, (uint64_t)addr, ret);
> -    *data = ret;
> -    return MEMTX_OK;
> -}
> -
> -static void send_ipi_data(CPULoongArchState *env, uint64_t val, hwaddr addr,
> -                          MemTxAttrs attrs)
> -{
> -    int i, mask = 0, data = 0;
> -
> -    /*
> -     * bit 27-30 is mask for byte writing,
> -     * if the mask is 0, we need not to do anything.
> -     */
> -    if ((val >> 27) & 0xf) {
> -        data = address_space_ldl(env->address_space_iocsr, addr,
> -                                 attrs, NULL);
> -        for (i = 0; i < 4; i++) {
> -            /* get mask for byte writing */
> -            if (val & (0x1 << (27 + i))) {
> -                mask |= 0xff << (i * 8);
> -            }
> -        }
> -    }
> -
> -    data &= mask;
> -    data |= (val >> 32) & ~mask;
> -    address_space_stl(env->address_space_iocsr, addr,
> -                      data, attrs, NULL);
> -}
> -
> -static int archid_cmp(const void *a, const void *b)
> -{
> -   CPUArchId *archid_a = (CPUArchId *)a;
> -   CPUArchId *archid_b = (CPUArchId *)b;
> -
> -   return archid_a->arch_id - archid_b->arch_id;
> -}
> -
> -static CPUArchId *find_cpu_by_archid(MachineState *ms, uint32_t id)
> -{
> -    CPUArchId apic_id, *found_cpu;
> -
> -    apic_id.arch_id = id;
> -    found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
> -        ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
> -        archid_cmp);
> -
> -    return found_cpu;
> -}
> -
> -static CPUState *ipi_getcpu(int arch_id)
> -{
> -    MachineState *machine = MACHINE(qdev_get_machine());
> -    CPUArchId *archid;
> -
> -    archid = find_cpu_by_archid(machine, arch_id);
> -    if (archid) {
> -        return CPU(archid->cpu);
> -    }
> -
> -    return NULL;
> -}
> -
> -static MemTxResult mail_send(uint64_t val, MemTxAttrs attrs)
> -{
> -    uint32_t cpuid;
> -    hwaddr addr;
> -    CPUState *cs;
> -
> -    cpuid = extract32(val, 16, 10);
> -    cs = ipi_getcpu(cpuid);
> -    if (cs == NULL) {
> -        return MEMTX_DECODE_ERROR;
> -    }
> -
> -    /* override requester_id */
> -    addr = SMP_IPI_MAILBOX + CORE_BUF_20 + (val & 0x1c);
> -    attrs.requester_id = cs->cpu_index;
> -    send_ipi_data(&LOONGARCH_CPU(cs)->env, val, addr, attrs);
> -    return MEMTX_OK;
> -}
> -
> -static MemTxResult any_send(uint64_t val, MemTxAttrs attrs)
> -{
> -    uint32_t cpuid;
> -    hwaddr addr;
> -    CPUState *cs;
> -
> -    cpuid = extract32(val, 16, 10);
> -    cs = ipi_getcpu(cpuid);
> -    if (cs == NULL) {
> -        return MEMTX_DECODE_ERROR;
> -    }
> -
> -    /* override requester_id */
> -    addr = val & 0xffff;
> -    attrs.requester_id = cs->cpu_index;
> -    send_ipi_data(&LOONGARCH_CPU(cs)->env, val, addr, attrs);
> -    return MEMTX_OK;
> -}
> -
> -static MemTxResult loongarch_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
> -                                        unsigned size, MemTxAttrs attrs)
> -{
> -    LoongArchIPI *ipi = opaque;
> -    IPICore *s;
> -    int index = 0;
> -    uint32_t cpuid;
> -    uint8_t vector;
> -    CPUState *cs;
> -
> -    s = &ipi->cpu[attrs.requester_id];
> -    addr &= 0xff;
> -    trace_loongarch_ipi_write(size, (uint64_t)addr, val);
> -    switch (addr) {
> -    case CORE_STATUS_OFF:
> -        qemu_log_mask(LOG_GUEST_ERROR, "can not be written");
> -        break;
> -    case CORE_EN_OFF:
> -        s->en = val;
> -        break;
> -    case CORE_SET_OFF:
> -        s->status |= val;
> -        if (s->status != 0 && (s->status & s->en) != 0) {
> -            qemu_irq_raise(s->irq);
> -        }
> -        break;
> -    case CORE_CLEAR_OFF:
> -        s->status &= ~val;
> -        if (s->status == 0 && s->en != 0) {
> -            qemu_irq_lower(s->irq);
> -        }
> -        break;
> -    case CORE_BUF_20 ... CORE_BUF_38 + 4:
> -        index = (addr - CORE_BUF_20) >> 2;
> -        s->buf[index] = val;
> -        break;
> -    case IOCSR_IPI_SEND:
> -        cpuid = extract32(val, 16, 10);
> -        /* IPI status vector */
> -        vector = extract8(val, 0, 5);
> -        cs = ipi_getcpu(cpuid);
> -        if (cs == NULL) {
> -            return MEMTX_DECODE_ERROR;
> -        }
> -
> -        /* override requester_id */
> -        attrs.requester_id = cs->cpu_index;
> -        loongarch_ipi_writel(ipi, CORE_SET_OFF, BIT(vector), 4, attrs);
> -        break;
> -    default:
> -        qemu_log_mask(LOG_UNIMP, "invalid write: %x", (uint32_t)addr);
> -        break;
> -    }
> -
> -    return MEMTX_OK;
> -}
> -
> -static const MemoryRegionOps loongarch_ipi_ops = {
> -    .read_with_attrs = loongarch_ipi_readl,
> -    .write_with_attrs = loongarch_ipi_writel,
> -    .impl.min_access_size = 4,
> -    .impl.max_access_size = 4,
> -    .valid.min_access_size = 4,
> -    .valid.max_access_size = 8,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> -
> -/* mail send and any send only support writeq */
> -static MemTxResult loongarch_ipi_writeq(void *opaque, hwaddr addr, uint64_t val,
> -                                        unsigned size, MemTxAttrs attrs)
> -{
> -    MemTxResult ret = MEMTX_OK;
> -
> -    addr &= 0xfff;
> -    switch (addr) {
> -    case MAIL_SEND_OFFSET:
> -        ret = mail_send(val, attrs);
> -        break;
> -    case ANY_SEND_OFFSET:
> -        ret = any_send(val, attrs);
> -        break;
> -    default:
> -       break;
> -    }
> -
> -    return ret;
> -}
> -
> -static const MemoryRegionOps loongarch_ipi64_ops = {
> -    .write_with_attrs = loongarch_ipi_writeq,
> -    .impl.min_access_size = 8,
> -    .impl.max_access_size = 8,
> -    .valid.min_access_size = 8,
> -    .valid.max_access_size = 8,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> -
> -static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
> -{
> -    LoongArchIPI *s = LOONGARCH_IPI(dev);
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> -    int i;
> -
> -    if (s->num_cpu == 0) {
> -        error_setg(errp, "num-cpu must be at least 1");
> -        return;
> -    }
> -
> -    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), &loongarch_ipi_ops,
> -                          s, "loongarch_ipi_iocsr", 0x48);
> -
> -    /* loongarch_ipi_iocsr performs re-entrant IO through ipi_send */
> -    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
> -
> -    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
> -
> -    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
> -                          &loongarch_ipi64_ops,
> -                          s, "loongarch_ipi64_iocsr", 0x118);
> -    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
It is different with existing implementation.

With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, however on 
loongarch ipi machine, there is no ipi_mmio_mem memory region.

So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory 
regions. In function sysbus_init_mmio(), memory region can not exceed
QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
region search speed also.

void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
{
     int n;

     assert(dev->num_mmio < QDEV_MAX_MMIO);
     n = dev->num_mmio++;
     dev->mmio[n].addr = -1;
     dev->mmio[n].memory = memory;
}

Can we revert this patch? We want to do production usable by real users 
rather than show pure technology.

Regards
Bibo Mao

> -
> -    s->cpu = g_new0(IPICore, s->num_cpu);
> -    if (s->cpu == NULL) {
> -        error_setg(errp, "Memory allocation for ExtIOICore faile");
> -        return;
> -    }
> -
> -    for (i = 0; i < s->num_cpu; i++) {
> -        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
> -    }
> -}
> -
> -static const VMStateDescription vmstate_ipi_core = {
> -    .name = "ipi-single",
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> -    .fields = (const VMStateField[]) {
> -        VMSTATE_UINT32(status, IPICore),
> -        VMSTATE_UINT32(en, IPICore),
> -        VMSTATE_UINT32(set, IPICore),
> -        VMSTATE_UINT32(clear, IPICore),
> -        VMSTATE_UINT32_ARRAY(buf, IPICore, IPI_MBX_NUM * 2),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
> -static const VMStateDescription vmstate_loongarch_ipi = {
> -    .name = TYPE_LOONGARCH_IPI,
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> -    .fields = (const VMStateField[]) {
> -        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, LoongArchIPI, num_cpu,
> -                         vmstate_ipi_core, IPICore),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
> -static Property ipi_properties[] = {
> -    DEFINE_PROP_UINT32("num-cpu", LoongArchIPI, num_cpu, 1),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -
> -    dc->realize = loongarch_ipi_realize;
> -    device_class_set_props(dc, ipi_properties);
> -    dc->vmsd = &vmstate_loongarch_ipi;
> -}
> -
> -static void loongarch_ipi_finalize(Object *obj)
> -{
> -    LoongArchIPI *s = LOONGARCH_IPI(obj);
> -
> -    g_free(s->cpu);
> -}
> -
> -static const TypeInfo loongarch_ipi_info = {
> -    .name          = TYPE_LOONGARCH_IPI,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(LoongArchIPI),
> -    .class_init    = loongarch_ipi_class_init,
> -    .instance_finalize = loongarch_ipi_finalize,
> -};
> -
> -static void loongarch_ipi_register_types(void)
> -{
> -    type_register_static(&loongarch_ipi_info);
> -}
> -
> -type_init(loongarch_ipi_register_types)
> 

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Jiaxun Yang]

在2024年6月26日六月 上午5:11，maobibo写道：
[...]
> It is different with existing implementation.
What do you mean? Isn't this the actual hardware behaviour?

>
> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, however on 
> loongarch ipi machine, there is no ipi_mmio_mem memory region.
> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory 
> regions. In function sysbus_init_mmio(), memory region can not exceed
> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
> region search speed also.
Ouch, never thought about that before, but I think we can control the
registration of sysbus_mmio with a device property or even ifdef so LoongArch
machine won't be affected.

For MIPS loongson3-virt machine, our CPU number is capped, so that won't
be a problem.

I'm currently travelling without access to my PC, I'll prepare a patch
as soon as I gain access again. Feel free to send a patch before me with
this approach if you desperately want to fix it.

>
> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
> {
>      int n;
>
>      assert(dev->num_mmio < QDEV_MAX_MMIO);
>      n = dev->num_mmio++;
>      dev->mmio[n].addr = -1;
>      dev->mmio[n].memory = memory;
> }
>
> Can we revert this patch? We want to do production usable by real users 
> rather than show pure technology.

I don't really get your point, we have at leat 4 real users requesting SMP
support for loongson3-virt on gitlab issues, plus I need this to test
MIPS/Loongson64 SMP kernel.

If there is a problem with your use case, we can fix it. Why we do want to
remove the functionality when there is an easy fix?

It’s not only the features necessary for you that made QEMU an outstanding
project; it’s everything coming together that completes it.

Thanks
- Jiaxun
>
> Regards
> Bibo Mao
>
>> -
>> -    s->cpu = g_new0(IPICore, s->num_cpu);
>> -    if (s->cpu == NULL) {
>> -        error_setg(errp, "Memory allocation for ExtIOICore faile");
>> -        return;
>> -    }
>> -
>> -    for (i = 0; i < s->num_cpu; i++) {
>> -        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
>> -    }
>> -}
>> -
>> -static const VMStateDescription vmstate_ipi_core = {
>> -    .name = "ipi-single",
>> -    .version_id = 2,
>> -    .minimum_version_id = 2,
>> -    .fields = (const VMStateField[]) {
>> -        VMSTATE_UINT32(status, IPICore),
>> -        VMSTATE_UINT32(en, IPICore),
>> -        VMSTATE_UINT32(set, IPICore),
>> -        VMSTATE_UINT32(clear, IPICore),
>> -        VMSTATE_UINT32_ARRAY(buf, IPICore, IPI_MBX_NUM * 2),
>> -        VMSTATE_END_OF_LIST()
>> -    }
>> -};
>> -
>> -static const VMStateDescription vmstate_loongarch_ipi = {
>> -    .name = TYPE_LOONGARCH_IPI,
>> -    .version_id = 2,
>> -    .minimum_version_id = 2,
>> -    .fields = (const VMStateField[]) {
>> -        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, LoongArchIPI, num_cpu,
>> -                         vmstate_ipi_core, IPICore),
>> -        VMSTATE_END_OF_LIST()
>> -    }
>> -};
>> -
>> -static Property ipi_properties[] = {
>> -    DEFINE_PROP_UINT32("num-cpu", LoongArchIPI, num_cpu, 1),
>> -    DEFINE_PROP_END_OF_LIST(),
>> -};
>> -
>> -static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>> -
>> -    dc->realize = loongarch_ipi_realize;
>> -    device_class_set_props(dc, ipi_properties);
>> -    dc->vmsd = &vmstate_loongarch_ipi;
>> -}
>> -
>> -static void loongarch_ipi_finalize(Object *obj)
>> -{
>> -    LoongArchIPI *s = LOONGARCH_IPI(obj);
>> -
>> -    g_free(s->cpu);
>> -}
>> -
>> -static const TypeInfo loongarch_ipi_info = {
>> -    .name          = TYPE_LOONGARCH_IPI,
>> -    .parent        = TYPE_SYS_BUS_DEVICE,
>> -    .instance_size = sizeof(LoongArchIPI),
>> -    .class_init    = loongarch_ipi_class_init,
>> -    .instance_finalize = loongarch_ipi_finalize,
>> -};
>> -
>> -static void loongarch_ipi_register_types(void)
>> -{
>> -    type_register_static(&loongarch_ipi_info);
>> -}
>> -
>> -type_init(loongarch_ipi_register_types)
>>

-- 
- Jiaxun

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[maobibo]

On 2024/6/26 下午3:40, Jiaxun Yang wrote:
> 
> 
> 在2024年6月26日六月 上午5:11，maobibo写道：
> [...]
>> It is different with existing implementation.
> What do you mean? Isn't this the actual hardware behaviour?
> 
>>
>> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, however on
>> loongarch ipi machine, there is no ipi_mmio_mem memory region.
>> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory
>> regions. In function sysbus_init_mmio(), memory region can not exceed
>> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
>> region search speed also.
> Ouch, never thought about that before, but I think we can control the
> registration of sysbus_mmio with a device property or even ifdef so LoongArch
> machine won't be affected.
> 
> For MIPS loongson3-virt machine, our CPU number is capped, so that won't
> be a problem.
> 
> I'm currently travelling without access to my PC, I'll prepare a patch
> as soon as I gain access again. Feel free to send a patch before me with
> this approach if you desperately want to fix it.
> 
>>
>> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
>> {
>>       int n;
>>
>>       assert(dev->num_mmio < QDEV_MAX_MMIO);
>>       n = dev->num_mmio++;
>>       dev->mmio[n].addr = -1;
>>       dev->mmio[n].memory = memory;
>> }
>>
>> Can we revert this patch? We want to do production usable by real users
>> rather than show pure technology.
> 
> I don't really get your point, we have at leat 4 real users requesting SMP
> support for loongson3-virt on gitlab issues, plus I need this to test
> MIPS/Loongson64 SMP kernel.
> 
> If there is a problem with your use case, we can fix it. Why we do want to
> remove the functionality when there is an easy fix?
I do not think we have the ability to abstract hw and continuous 
maintenance for two different architecture, including you and me.

So I suggest that different files will be better for the present. After 
one year or later, if we have further understanding about system, it is 
ok to merge them into one file.

Regards
Bibo Mao
> 
> It’s not only the features necessary for you that made QEMU an outstanding
> project; it’s everything coming together that completes it.
> 
> Thanks
> - Jiaxun
>>
>> Regards
>> Bibo Mao
>>
>>> -
>>> -    s->cpu = g_new0(IPICore, s->num_cpu);
>>> -    if (s->cpu == NULL) {
>>> -        error_setg(errp, "Memory allocation for ExtIOICore faile");
>>> -        return;
>>> -    }
>>> -
>>> -    for (i = 0; i < s->num_cpu; i++) {
>>> -        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
>>> -    }
>>> -}
>>> -
>>> -static const VMStateDescription vmstate_ipi_core = {
>>> -    .name = "ipi-single",
>>> -    .version_id = 2,
>>> -    .minimum_version_id = 2,
>>> -    .fields = (const VMStateField[]) {
>>> -        VMSTATE_UINT32(status, IPICore),
>>> -        VMSTATE_UINT32(en, IPICore),
>>> -        VMSTATE_UINT32(set, IPICore),
>>> -        VMSTATE_UINT32(clear, IPICore),
>>> -        VMSTATE_UINT32_ARRAY(buf, IPICore, IPI_MBX_NUM * 2),
>>> -        VMSTATE_END_OF_LIST()
>>> -    }
>>> -};
>>> -
>>> -static const VMStateDescription vmstate_loongarch_ipi = {
>>> -    .name = TYPE_LOONGARCH_IPI,
>>> -    .version_id = 2,
>>> -    .minimum_version_id = 2,
>>> -    .fields = (const VMStateField[]) {
>>> -        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, LoongArchIPI, num_cpu,
>>> -                         vmstate_ipi_core, IPICore),
>>> -        VMSTATE_END_OF_LIST()
>>> -    }
>>> -};
>>> -
>>> -static Property ipi_properties[] = {
>>> -    DEFINE_PROP_UINT32("num-cpu", LoongArchIPI, num_cpu, 1),
>>> -    DEFINE_PROP_END_OF_LIST(),
>>> -};
>>> -
>>> -static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
>>> -{
>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>> -
>>> -    dc->realize = loongarch_ipi_realize;
>>> -    device_class_set_props(dc, ipi_properties);
>>> -    dc->vmsd = &vmstate_loongarch_ipi;
>>> -}
>>> -
>>> -static void loongarch_ipi_finalize(Object *obj)
>>> -{
>>> -    LoongArchIPI *s = LOONGARCH_IPI(obj);
>>> -
>>> -    g_free(s->cpu);
>>> -}
>>> -
>>> -static const TypeInfo loongarch_ipi_info = {
>>> -    .name          = TYPE_LOONGARCH_IPI,
>>> -    .parent        = TYPE_SYS_BUS_DEVICE,
>>> -    .instance_size = sizeof(LoongArchIPI),
>>> -    .class_init    = loongarch_ipi_class_init,
>>> -    .instance_finalize = loongarch_ipi_finalize,
>>> -};
>>> -
>>> -static void loongarch_ipi_register_types(void)
>>> -{
>>> -    type_register_static(&loongarch_ipi_info);
>>> -}
>>> -
>>> -type_init(loongarch_ipi_register_types)
>>>
> 

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Jiaxun Yang]

在2024年6月26日六月 上午8:51，maobibo写道：
> On 2024/6/26 下午3:40, Jiaxun Yang wrote:
>> 
>> 
>> 在2024年6月26日六月 上午5:11，maobibo写道：
>> [...]
>>> It is different with existing implementation.
>> What do you mean? Isn't this the actual hardware behaviour?
>> 
>>>
>>> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, however on
>>> loongarch ipi machine, there is no ipi_mmio_mem memory region.
>>> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory
>>> regions. In function sysbus_init_mmio(), memory region can not exceed
>>> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
>>> region search speed also.
>> Ouch, never thought about that before, but I think we can control the
>> registration of sysbus_mmio with a device property or even ifdef so LoongArch
>> machine won't be affected.
>> 
>> For MIPS loongson3-virt machine, our CPU number is capped, so that won't
>> be a problem.
>> 
>> I'm currently travelling without access to my PC, I'll prepare a patch
>> as soon as I gain access again. Feel free to send a patch before me with
>> this approach if you desperately want to fix it.
>> 
>>>
>>> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
>>> {
>>>       int n;
>>>
>>>       assert(dev->num_mmio < QDEV_MAX_MMIO);
>>>       n = dev->num_mmio++;
>>>       dev->mmio[n].addr = -1;
>>>       dev->mmio[n].memory = memory;
>>> }
>>>
>>> Can we revert this patch? We want to do production usable by real users
>>> rather than show pure technology.
>> 
>> I don't really get your point, we have at leat 4 real users requesting SMP
>> support for loongson3-virt on gitlab issues, plus I need this to test
>> MIPS/Loongson64 SMP kernel.
>> 
>> If there is a problem with your use case, we can fix it. Why we do want to
>> remove the functionality when there is an easy fix?
> I do not think we have the ability to abstract hw and continuous 
> maintenance for two different architecture, including you and me.

After all it’s the same IP block, I fail to see any reason to implement it separately.

I perfectly understand that piece of hardware IP and I fix broken stuff in this driver time by time.

I understand how does it work on LoongArch systems so I make my best effort to cover LoongArch part.

There might be some edge cases that I missed, and I’ll be thankful for reports or assistance.

The approach of shared drivers for LoongArch/MIPS works so well in kernel development, for QEMU I believe it’s also desirable.

I’m not asking you or Loongson to maintain any MIPS/Loongson related features, I’ll take care of them if things go wrong.

>lp
> So I suggest that different files will be better for the present. After 
> one year or later, if we have further understanding about system, it is 
> ok to merge them into one file.

If there is anything not clear to you on both MIPS and LoongArch side please let me know, I’m happy to help.

We promote code reuse in QEMU to minimize maintenance burden.

Thanks

>
> Regards
> Bibo Mao
>> 
>> It’s not only the features necessary for you that made QEMU an outstanding
>> project; it’s everything coming together that completes it.
>> 
>> Thanks
>> - Jiaxun
>>>
>>> Regards
>>> Bibo Mao
>>>
>>>> -
>>>> -    s->cpu = g_new0(IPICore, s->num_cpu);
>>>> -    if (s->cpu == NULL) {
>>>> -        error_setg(errp, "Memory allocation for ExtIOICore faile");
>>>> -        return;
>>>> -    }
>>>> -
>>>> -    for (i = 0; i < s->num_cpu; i++) {
>>>> -        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
>>>> -    }
>>>> -}
>>>> -
>>>> -static const VMStateDescription vmstate_ipi_core = {
>>>> -    .name = "ipi-single",
>>>> -    .version_id = 2,
>>>> -    .minimum_version_id = 2,
>>>> -    .fields = (const VMStateField[]) {
>>>> -        VMSTATE_UINT32(status, IPICore),
>>>> -        VMSTATE_UINT32(en, IPICore),
>>>> -        VMSTATE_UINT32(set, IPICore),
>>>> -        VMSTATE_UINT32(clear, IPICore),
>>>> -        VMSTATE_UINT32_ARRAY(buf, IPICore, IPI_MBX_NUM * 2),
>>>> -        VMSTATE_END_OF_LIST()
>>>> -    }
>>>> -};
>>>> -
>>>> -static const VMStateDescription vmstate_loongarch_ipi = {
>>>> -    .name = TYPE_LOONGARCH_IPI,
>>>> -    .version_id = 2,
>>>> -    .minimum_version_id = 2,
>>>> -    .fields = (const VMStateField[]) {
>>>> -        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, LoongArchIPI, num_cpu,
>>>> -                         vmstate_ipi_core, IPICore),
>>>> -        VMSTATE_END_OF_LIST()
>>>> -    }
>>>> -};
>>>> -
>>>> -static Property ipi_properties[] = {
>>>> -    DEFINE_PROP_UINT32("num-cpu", LoongArchIPI, num_cpu, 1),
>>>> -    DEFINE_PROP_END_OF_LIST(),
>>>> -};
>>>> -
>>>> -static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
>>>> -{
>>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> -
>>>> -    dc->realize = loongarch_ipi_realize;
>>>> -    device_class_set_props(dc, ipi_properties);
>>>> -    dc->vmsd = &vmstate_loongarch_ipi;
>>>> -}
>>>> -
>>>> -static void loongarch_ipi_finalize(Object *obj)
>>>> -{
>>>> -    LoongArchIPI *s = LOONGARCH_IPI(obj);
>>>> -
>>>> -    g_free(s->cpu);
>>>> -}
>>>> -
>>>> -static const TypeInfo loongarch_ipi_info = {
>>>> -    .name          = TYPE_LOONGARCH_IPI,
>>>> -    .parent        = TYPE_SYS_BUS_DEVICE,
>>>> -    .instance_size = sizeof(LoongArchIPI),
>>>> -    .class_init    = loongarch_ipi_class_init,
>>>> -    .instance_finalize = loongarch_ipi_finalize,
>>>> -};
>>>> -
>>>> -static void loongarch_ipi_register_types(void)
>>>> -{
>>>> -    type_register_static(&loongarch_ipi_info);
>>>> -}
>>>> -
>>>> -type_init(loongarch_ipi_register_types)
>>>>
>>

-- 
- Jiaxun

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Philippe Mathieu-Daudé]

Hi Bibo,

On 26/6/24 06:11, maobibo wrote:
> 
> 
> On 2024/6/5 上午10:15, Jiaxun Yang wrote:
>> It was missed out in previous commit.
>>
>> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>   hw/intc/loongarch_ipi.c | 347 
>> ------------------------------------------------
>>   1 file changed, 347 deletions(-)


>> -static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
>> -{
>> -    LoongArchIPI *s = LOONGARCH_IPI(dev);
>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> -    int i;
>> -
>> -    if (s->num_cpu == 0) {
>> -        error_setg(errp, "num-cpu must be at least 1");
>> -        return;
>> -    }
>> -
>> -    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), 
>> &loongarch_ipi_ops,
>> -                          s, "loongarch_ipi_iocsr", 0x48);
>> -
>> -    /* loongarch_ipi_iocsr performs re-entrant IO through ipi_send */
>> -    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
>> -
>> -    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
>> -
>> -    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
>> -                          &loongarch_ipi64_ops,
>> -                          s, "loongarch_ipi64_iocsr", 0x118);
>> -    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
> It is different with existing implementation.
> 
> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, however on 
> loongarch ipi machine, there is no ipi_mmio_mem memory region.
> 
> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory 
> regions. In function sysbus_init_mmio(), memory region can not exceed
> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
> region search speed also.
> 
> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
> {
>      int n;
> 
>      assert(dev->num_mmio < QDEV_MAX_MMIO);
>      n = dev->num_mmio++;
>      dev->mmio[n].addr = -1;
>      dev->mmio[n].memory = memory;
> }
> 
> Can we revert this patch? We want to do production usable by real users 
> rather than show pure technology.

Since commit b4a12dfc2132 this file is not built/tested anymore:

-specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true: 
files('loongarch_ipi.c'))
+specific_ss.add(when: 'CONFIG_LOONGSON_IPI', if_true: 
files('loongson_ipi.c'))

We don't want to maintain dead code.

Regards,

Phil.

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[gaosong]

在 2024/6/26 下午8:10, Philippe Mathieu-Daudé 写道:
> Hi Bibo,
>
> On 26/6/24 06:11, maobibo wrote:
>>
>>
>> On 2024/6/5 上午10:15, Jiaxun Yang wrote:
>>> It was missed out in previous commit.
>>>
>>> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>   hw/intc/loongarch_ipi.c | 347 
>>> ------------------------------------------------
>>>   1 file changed, 347 deletions(-)
>
>
>>> -static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
>>> -{
>>> -    LoongArchIPI *s = LOONGARCH_IPI(dev);
>>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> -    int i;
>>> -
>>> -    if (s->num_cpu == 0) {
>>> -        error_setg(errp, "num-cpu must be at least 1");
>>> -        return;
>>> -    }
>>> -
>>> -    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), 
>>> &loongarch_ipi_ops,
>>> -                          s, "loongarch_ipi_iocsr", 0x48);
>>> -
>>> -    /* loongarch_ipi_iocsr performs re-entrant IO through ipi_send */
>>> -    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
>>> -
>>> -    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
>>> -
>>> -    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
>>> -                          &loongarch_ipi64_ops,
>>> -                          s, "loongarch_ipi64_iocsr", 0x118);
>>> -    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
>> It is different with existing implementation.
>>
>> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, however 
>> on loongarch ipi machine, there is no ipi_mmio_mem memory region.
>>
>> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory 
>> regions. In function sysbus_init_mmio(), memory region can not exceed
>> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
>> region search speed also.
>>
>> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
>> {
>>      int n;
>>
>>      assert(dev->num_mmio < QDEV_MAX_MMIO);
>>      n = dev->num_mmio++;
>>      dev->mmio[n].addr = -1;
>>      dev->mmio[n].memory = memory;
>> }
>>
>> Can we revert this patch? We want to do production usable by real 
>> users rather than show pure technology.
>
> Since commit b4a12dfc2132 this file is not built/tested anymore:
>
> -specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true: 
> files('loongarch_ipi.c'))
> +specific_ss.add(when: 'CONFIG_LOONGSON_IPI', if_true: 
> files('loongson_ipi.c'))
>
> We don't want to maintain dead code.
>
Hi,  Philippe

It is commmit 49eba52a5 that causes Loongarch to fail to start.

What bibao means is that LoongArch and mips do not share "lloongson_ipi.c".
This avoids mutual influence.


My understanding of the next sentence is as follows.

Nowadays, most of the open source operating systems in China use the 
latest QEMU.
e.g. OpenEuler/OpenAnolis/OpenCloudOS, etc. These operating systems have 
a large
  number of real users. so we need to maintain the stability of the 
LoongArch architecture
of the QEMU community as much as possible. This will reduce maintenance 
costs.

Therefore, we would like to restore the 'loongarch_ipi.c' file. what do 
you think?

Thanks.
Song Gao

> Regards,
>
> Phil.

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Philippe Mathieu-Daudé]

On 27/6/24 04:44, gaosong wrote:
> 在 2024/6/26 下午8:10, Philippe Mathieu-Daudé 写道:
>> Hi Bibo,
>>
>> On 26/6/24 06:11, maobibo wrote:
>>>
>>>
>>> On 2024/6/5 上午10:15, Jiaxun Yang wrote:
>>>> It was missed out in previous commit.
>>>>
>>>> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>> ---
>>>>   hw/intc/loongarch_ipi.c | 347 
>>>> ------------------------------------------------
>>>>   1 file changed, 347 deletions(-)
>>
>>
>>>> -static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
>>>> -{
>>>> -    LoongArchIPI *s = LOONGARCH_IPI(dev);
>>>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>> -    int i;
>>>> -
>>>> -    if (s->num_cpu == 0) {
>>>> -        error_setg(errp, "num-cpu must be at least 1");
>>>> -        return;
>>>> -    }
>>>> -
>>>> -    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), 
>>>> &loongarch_ipi_ops,
>>>> -                          s, "loongarch_ipi_iocsr", 0x48);
>>>> -
>>>> -    /* loongarch_ipi_iocsr performs re-entrant IO through ipi_send */
>>>> -    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
>>>> -
>>>> -    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
>>>> -
>>>> -    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
>>>> -                          &loongarch_ipi64_ops,
>>>> -                          s, "loongarch_ipi64_iocsr", 0x118);
>>>> -    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
>>> It is different with existing implementation.
>>>
>>> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, however 
>>> on loongarch ipi machine, there is no ipi_mmio_mem memory region.
>>>
>>> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory 
>>> regions. In function sysbus_init_mmio(), memory region can not exceed
>>> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
>>> region search speed also.
>>>
>>> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
>>> {
>>>      int n;
>>>
>>>      assert(dev->num_mmio < QDEV_MAX_MMIO);
>>>      n = dev->num_mmio++;
>>>      dev->mmio[n].addr = -1;
>>>      dev->mmio[n].memory = memory;
>>> }
>>>
>>> Can we revert this patch? We want to do production usable by real 
>>> users rather than show pure technology.
>>
>> Since commit b4a12dfc2132 this file is not built/tested anymore:
>>
>> -specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true: 
>> files('loongarch_ipi.c'))
>> +specific_ss.add(when: 'CONFIG_LOONGSON_IPI', if_true: 
>> files('loongson_ipi.c'))
>>
>> We don't want to maintain dead code.
>>
> Hi,  Philippe
> 
> It is commmit 49eba52a5 that causes Loongarch to fail to start.
> 
> What bibao means is that LoongArch and mips do not share "lloongson_ipi.c".
> This avoids mutual influence.
> 
> 
> My understanding of the next sentence is as follows.
> 
> Nowadays, most of the open source operating systems in China use the 
> latest QEMU.
> e.g. OpenEuler/OpenAnolis/OpenCloudOS, etc. These operating systems have 
> a large
>   number of real users. so we need to maintain the stability of the 
> LoongArch architecture
> of the QEMU community as much as possible. This will reduce maintenance 
> costs.

I'm glad there is a such large number of users :)

> Therefore, we would like to restore the 'loongarch_ipi.c' file. what do 
> you think?

My preference on "reducing maintenance cost" is code reuse instead of
duplication.

Before reverting, lets try to fix the issue. I suggested a v2:
https://lore.kernel.org/qemu-devel/20240627125819.62779-2-philmd@linaro.org

That said, both current patch and the suggested fix pass our
Avocado CI test suite (running tests/avocado/machine_loongarch.py).

Is your use case not covered? Could you expand the CI tests so
we don't hit this problem again? (Also we could reproduce and
fix more easily).

Thanks,

Phil.

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[maobibo]

Hi Philippe,

On 2024/6/27 下午9:02, Philippe Mathieu-Daudé wrote:
> On 27/6/24 04:44, gaosong wrote:
>> 在 2024/6/26 下午8:10, Philippe Mathieu-Daudé 写道:
>>> Hi Bibo,
>>>
>>> On 26/6/24 06:11, maobibo wrote:
>>>>
>>>>
>>>> On 2024/6/5 上午10:15, Jiaxun Yang wrote:
>>>>> It was missed out in previous commit.
>>>>>
>>>>> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
>>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>>> ---
>>>>>   hw/intc/loongarch_ipi.c | 347 
>>>>> ------------------------------------------------
>>>>>   1 file changed, 347 deletions(-)
>>>
>>>
>>>>> -static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
>>>>> -{
>>>>> -    LoongArchIPI *s = LOONGARCH_IPI(dev);
>>>>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>>> -    int i;
>>>>> -
>>>>> -    if (s->num_cpu == 0) {
>>>>> -        error_setg(errp, "num-cpu must be at least 1");
>>>>> -        return;
>>>>> -    }
>>>>> -
>>>>> -    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), 
>>>>> &loongarch_ipi_ops,
>>>>> -                          s, "loongarch_ipi_iocsr", 0x48);
>>>>> -
>>>>> -    /* loongarch_ipi_iocsr performs re-entrant IO through ipi_send */
>>>>> -    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
>>>>> -
>>>>> -    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
>>>>> -
>>>>> -    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
>>>>> -                          &loongarch_ipi64_ops,
>>>>> -                          s, "loongarch_ipi64_iocsr", 0x118);
>>>>> -    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
>>>> It is different with existing implementation.
>>>>
>>>> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, 
>>>> however on loongarch ipi machine, there is no ipi_mmio_mem memory 
>>>> region.
>>>>
>>>> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory 
>>>> regions. In function sysbus_init_mmio(), memory region can not exceed
>>>> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
>>>> region search speed also.
>>>>
>>>> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
>>>> {
>>>>      int n;
>>>>
>>>>      assert(dev->num_mmio < QDEV_MAX_MMIO);
>>>>      n = dev->num_mmio++;
>>>>      dev->mmio[n].addr = -1;
>>>>      dev->mmio[n].memory = memory;
>>>> }
>>>>
>>>> Can we revert this patch? We want to do production usable by real 
>>>> users rather than show pure technology.
>>>
>>> Since commit b4a12dfc2132 this file is not built/tested anymore:
>>>
>>> -specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true: 
>>> files('loongarch_ipi.c'))
>>> +specific_ss.add(when: 'CONFIG_LOONGSON_IPI', if_true: 
>>> files('loongson_ipi.c'))
>>>
>>> We don't want to maintain dead code.
>>>
>> Hi,  Philippe
>>
>> It is commmit 49eba52a5 that causes Loongarch to fail to start.
>>
>> What bibao means is that LoongArch and mips do not share 
>> "lloongson_ipi.c".
>> This avoids mutual influence.
>>
>>
>> My understanding of the next sentence is as follows.
>>
>> Nowadays, most of the open source operating systems in China use the 
>> latest QEMU.
>> e.g. OpenEuler/OpenAnolis/OpenCloudOS, etc. These operating systems 
>> have a large
>>   number of real users. so we need to maintain the stability of the 
>> LoongArch architecture
>> of the QEMU community as much as possible. This will reduce 
>> maintenance costs.
> 
> I'm glad there is a such large number of users :)
> 
>> Therefore, we would like to restore the 'loongarch_ipi.c' file. what 
>> do you think?
> 
> My preference on "reducing maintenance cost" is code reuse instead of
> duplication.
> 
> Before reverting, lets try to fix the issue. I suggested a v2:
> https://lore.kernel.org/qemu-devel/20240627125819.62779-2-philmd@linaro.org
Sorry for late reply.

How about split loongson_ipi.c into 
loongson_ipi_base.c/loongson_ipi_loongson.c/loongson_ipi_loongarch.c,

File loongson_ipi_base.c contains the common code, loongson_ipi_xxx.c 
contains arch specific. Soon we will submit irqchip in kernel function,
it will be much different for architectures, since ioctl command is 
different for two architectures to save/restore ipi registers.

Regards
Bibo Mao

> 
> That said, both current patch and the suggested fix pass our
> Avocado CI test suite (running tests/avocado/machine_loongarch.py).
> 
> Is your use case not covered? Could you expand the CI tests so
> we don't hit this problem again? (Also we could reproduce and
> fix more easily).
> 
> Thanks,
> 
> Phil.

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[maobibo]

Also this patch is problematic on LoongArch.

The original patch is to search physical cpuid rather than logic cpuid.

We want to make ipi module better and better, however now it comes back 
to initial state at the beginning :(

commit 03ca348b6b9038ce284916b36c19f700ac0ce7a6
Author: Jiaxun Yang <jiaxun.yang@flygoat.com>
Date:   Wed Jun 5 10:04:27 2024

     hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id

     cpu_by_arch_id is doing the same thing as our ipi_getcpu logic.

     Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
     Reviewed-by: Song Gao <gaosong@loongson.cn>
     Message-ID: <20240605-loongson3-ipi-v3-4-ddd2c0e03fa3@flygoat.com>
     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Regards
Bibo Mao

On 2024/7/1 上午9:35, maobibo wrote:
> Hi Philippe,
> 
> On 2024/6/27 下午9:02, Philippe Mathieu-Daudé wrote:
>> On 27/6/24 04:44, gaosong wrote:
>>> 在 2024/6/26 下午8:10, Philippe Mathieu-Daudé 写道:
>>>> Hi Bibo,
>>>>
>>>> On 26/6/24 06:11, maobibo wrote:
>>>>>
>>>>>
>>>>> On 2024/6/5 上午10:15, Jiaxun Yang wrote:
>>>>>> It was missed out in previous commit.
>>>>>>
>>>>>> Fixes: b4a12dfc2132 ("hw/intc/loongarch_ipi: Rename as loongson_ipi")
>>>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>>>> ---
>>>>>>   hw/intc/loongarch_ipi.c | 347 
>>>>>> ------------------------------------------------
>>>>>>   1 file changed, 347 deletions(-)
>>>>
>>>>
>>>>>> -static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
>>>>>> -{
>>>>>> -    LoongArchIPI *s = LOONGARCH_IPI(dev);
>>>>>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>>>> -    int i;
>>>>>> -
>>>>>> -    if (s->num_cpu == 0) {
>>>>>> -        error_setg(errp, "num-cpu must be at least 1");
>>>>>> -        return;
>>>>>> -    }
>>>>>> -
>>>>>> -    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), 
>>>>>> &loongarch_ipi_ops,
>>>>>> -                          s, "loongarch_ipi_iocsr", 0x48);
>>>>>> -
>>>>>> -    /* loongarch_ipi_iocsr performs re-entrant IO through 
>>>>>> ipi_send */
>>>>>> -    s->ipi_iocsr_mem.disable_reentrancy_guard = true;
>>>>>> -
>>>>>> -    sysbus_init_mmio(sbd, &s->ipi_iocsr_mem);
>>>>>> -
>>>>>> -    memory_region_init_io(&s->ipi64_iocsr_mem, OBJECT(dev),
>>>>>> -                          &loongarch_ipi64_ops,
>>>>>> -                          s, "loongarch_ipi64_iocsr", 0x118);
>>>>>> -    sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
>>>>> It is different with existing implementation.
>>>>>
>>>>> With hw/intc/loongson_ipi.c, every vcpu has one ipi_mmio_mem, 
>>>>> however on loongarch ipi machine, there is no ipi_mmio_mem memory 
>>>>> region.
>>>>>
>>>>> So if machine has 256 vcpus, there will be 256 ipi_mmio_mem memory 
>>>>> regions. In function sysbus_init_mmio(), memory region can not exceed
>>>>> QDEV_MAX_MMIO (32).  With so many memory regions, it slows down memory
>>>>> region search speed also.
>>>>>
>>>>> void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
>>>>> {
>>>>>      int n;
>>>>>
>>>>>      assert(dev->num_mmio < QDEV_MAX_MMIO);
>>>>>      n = dev->num_mmio++;
>>>>>      dev->mmio[n].addr = -1;
>>>>>      dev->mmio[n].memory = memory;
>>>>> }
>>>>>
>>>>> Can we revert this patch? We want to do production usable by real 
>>>>> users rather than show pure technology.
>>>>
>>>> Since commit b4a12dfc2132 this file is not built/tested anymore:
>>>>
>>>> -specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true: 
>>>> files('loongarch_ipi.c'))
>>>> +specific_ss.add(when: 'CONFIG_LOONGSON_IPI', if_true: 
>>>> files('loongson_ipi.c'))
>>>>
>>>> We don't want to maintain dead code.
>>>>
>>> Hi,  Philippe
>>>
>>> It is commmit 49eba52a5 that causes Loongarch to fail to start.
>>>
>>> What bibao means is that LoongArch and mips do not share 
>>> "lloongson_ipi.c".
>>> This avoids mutual influence.
>>>
>>>
>>> My understanding of the next sentence is as follows.
>>>
>>> Nowadays, most of the open source operating systems in China use the 
>>> latest QEMU.
>>> e.g. OpenEuler/OpenAnolis/OpenCloudOS, etc. These operating systems 
>>> have a large
>>>   number of real users. so we need to maintain the stability of the 
>>> LoongArch architecture
>>> of the QEMU community as much as possible. This will reduce 
>>> maintenance costs.
>>
>> I'm glad there is a such large number of users :)
>>
>>> Therefore, we would like to restore the 'loongarch_ipi.c' file. what 
>>> do you think?
>>
>> My preference on "reducing maintenance cost" is code reuse instead of
>> duplication.
>>
>> Before reverting, lets try to fix the issue. I suggested a v2:
>> https://lore.kernel.org/qemu-devel/20240627125819.62779-2-philmd@linaro.org 
>>
> Sorry for late reply.
> 
> How about split loongson_ipi.c into 
> loongson_ipi_base.c/loongson_ipi_loongson.c/loongson_ipi_loongarch.c,
> 
> File loongson_ipi_base.c contains the common code, loongson_ipi_xxx.c 
> contains arch specific. Soon we will submit irqchip in kernel function,
> it will be much different for architectures, since ioctl command is 
> different for two architectures to save/restore ipi registers.
> 
> Regards
> Bibo Mao
> 
>>
>> That said, both current patch and the suggested fix pass our
>> Avocado CI test suite (running tests/avocado/machine_loongarch.py).
>>
>> Is your use case not covered? Could you expand the CI tests so
>> we don't hit this problem again? (Also we could reproduce and
>> fix more easily).
>>
>> Thanks,
>>
>> Phil.
> 

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Jiaxun Yang]

在2024年7月1日七月 上午7:44，maobibo写道：
> Also this patch is problematic on LoongArch.
>
> The original patch is to search physical cpuid rather than logic cpuid.
>
> We want to make ipi module better and better, however now it comes back 
> to initial state at the beginning :(

Isn't arch_id the "physical id" you want? "cs->cpu_index" is the logical ID
for QEMU.

arch_id is setup by arch code, like APIC ID for x86.

I had come across the old ipi_getcpu  implementation, and I'm sure we were
looking at arch_id as well.

Thanks
- Jiaxun
>
> commit 03ca348b6b9038ce284916b36c19f700ac0ce7a6
> Author: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Date:   Wed Jun 5 10:04:27 2024
>
>      hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id
>
>      cpu_by_arch_id is doing the same thing as our ipi_getcpu logic.
>
>      Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>      Reviewed-by: Song Gao <gaosong@loongson.cn>
>      Message-ID: <20240605-loongson3-ipi-v3-4-ddd2c0e03fa3@flygoat.com>
>      Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
> Regards
> Bibo Mao
>
-- 
- Jiaxun

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[maobibo]

On 2024/7/1 下午3:01, Jiaxun Yang wrote:
> 
> 
> 在2024年7月1日七月 上午7:44，maobibo写道：
>> Also this patch is problematic on LoongArch.
>>
>> The original patch is to search physical cpuid rather than logic cpuid.
>>
>> We want to make ipi module better and better, however now it comes back
>> to initial state at the beginning :(
> 
> Isn't arch_id the "physical id" you want? "cs->cpu_index" is the logical ID
> for QEMU.
> 
> arch_id is setup by arch code, like APIC ID for x86.
> 
> I had come across the old ipi_getcpu  implementation, and I'm sure we were
> looking at arch_id as well.
So, where is implementation code for function get_arch_id() looking for 
vcpu with physical index?

Regards
Bibo Mao

> 
> Thanks
> - Jiaxun
>>
>> commit 03ca348b6b9038ce284916b36c19f700ac0ce7a6
>> Author: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Date:   Wed Jun 5 10:04:27 2024
>>
>>       hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id
>>
>>       cpu_by_arch_id is doing the same thing as our ipi_getcpu logic.
>>
>>       Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>       Reviewed-by: Song Gao <gaosong@loongson.cn>
>>       Message-ID: <20240605-loongson3-ipi-v3-4-ddd2c0e03fa3@flygoat.com>
>>       Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>>
>> Regards
>> Bibo Mao
>>

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Jiaxun Yang]

在2024年7月1日七月 上午8:22，maobibo写道：
> On 2024/7/1 下午3:01, Jiaxun Yang wrote:
>> 
>> 
>> 在2024年7月1日七月 上午7:44，maobibo写道：
>>> Also this patch is problematic on LoongArch.
>>>
>>> The original patch is to search physical cpuid rather than logic cpuid.
>>>
>>> We want to make ipi module better and better, however now it comes back
>>> to initial state at the beginning :(
>> 
>> Isn't arch_id the "physical id" you want? "cs->cpu_index" is the logical ID
>> for QEMU.
>> 
>> arch_id is setup by arch code, like APIC ID for x86.
>> 
>> I had come across the old ipi_getcpu  implementation, and I'm sure we were
>> looking at arch_id as well.
> So, where is implementation code for function get_arch_id() looking for 
> vcpu with physical index?

Hi Bibo,

cpu_by_arch_id will be redirected to:

```
CPUState *cpu_by_arch_id(int64_t id)
{
    CPUState *cpu;

    CPU_FOREACH(cpu) {
        CPUClass *cc = CPU_GET_CLASS(cpu);

        if (cc->get_arch_id(cpu) == id) {
            return cpu;
        }
    }
    return NULL;
}
```

It iterates over all vcpus and return CPUStates with corresponding arch_id.

Whereas, for LoongArch's get_arch_id implementation:
```
static int64_t loongarch_cpu_get_arch_id(CPUState *cs)
{
    LoongArchCPU *cpu = LOONGARCH_CPU(cs);

    return cpu->phy_id;
}
```
I believe it matches our intension here.

Thanks

>
> Regards
> Bibo Mao
>
>> 
>> Thanks
>> - Jiaxun
>>>
>>> commit 03ca348b6b9038ce284916b36c19f700ac0ce7a6
>>> Author: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> Date:   Wed Jun 5 10:04:27 2024
>>>
>>>       hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id
>>>
>>>       cpu_by_arch_id is doing the same thing as our ipi_getcpu logic.
>>>
>>>       Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>       Reviewed-by: Song Gao <gaosong@loongson.cn>
>>>       Message-ID: <20240605-loongson3-ipi-v3-4-ddd2c0e03fa3@flygoat.com>
>>>       Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>>
>>> Regards
>>> Bibo Mao
>>>

-- 
- Jiaxun

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[maobibo]

On 2024/7/1 下午4:29, Jiaxun Yang wrote:
> 
> 
> 在2024年7月1日七月 上午8:22，maobibo写道：
>> On 2024/7/1 下午3:01, Jiaxun Yang wrote:
>>>
>>>
>>> 在2024年7月1日七月 上午7:44，maobibo写道：
>>>> Also this patch is problematic on LoongArch.
>>>>
>>>> The original patch is to search physical cpuid rather than logic cpuid.
>>>>
>>>> We want to make ipi module better and better, however now it comes back
>>>> to initial state at the beginning :(
>>>
>>> Isn't arch_id the "physical id" you want? "cs->cpu_index" is the logical ID
>>> for QEMU.
>>>
>>> arch_id is setup by arch code, like APIC ID for x86.
>>>
>>> I had come across the old ipi_getcpu  implementation, and I'm sure we were
>>> looking at arch_id as well.
>> So, where is implementation code for function get_arch_id() looking for
>> vcpu with physical index?
> 
> Hi Bibo,
> 
> cpu_by_arch_id will be redirected to:
> 
> ```
> CPUState *cpu_by_arch_id(int64_t id)
> {
>      CPUState *cpu;
> 
>      CPU_FOREACH(cpu) {
>          CPUClass *cc = CPU_GET_CLASS(cpu);
> 
>          if (cc->get_arch_id(cpu) == id) {
>              return cpu;
>          }
>      }
>      return NULL;
> }
> ```
> 
> It iterates over all vcpus and return CPUStates with corresponding arch_id.
> 
> Whereas, for LoongArch's get_arch_id implementation:
> ```
> static int64_t loongarch_cpu_get_arch_id(CPUState *cs)
> {
>      LoongArchCPU *cpu = LOONGARCH_CPU(cs);
> 
>      return cpu->phy_id;
> }
> ```
> I believe it matches our intension here.
yes, you are right.

Got it, I miss the architecture specific implementation, and thanks for 
pointing it out with patience.

Regards
> 
> Thanks
> 
>>
>> Regards
>> Bibo Mao
>>
>>>
>>> Thanks
>>> - Jiaxun
>>>>
>>>> commit 03ca348b6b9038ce284916b36c19f700ac0ce7a6
>>>> Author: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>> Date:   Wed Jun 5 10:04:27 2024
>>>>
>>>>        hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id
>>>>
>>>>        cpu_by_arch_id is doing the same thing as our ipi_getcpu logic.
>>>>
>>>>        Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>>        Reviewed-by: Song Gao <gaosong@loongson.cn>
>>>>        Message-ID: <20240605-loongson3-ipi-v3-4-ddd2c0e03fa3@flygoat.com>
>>>>        Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>
> 

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[Jiaxun Yang]

在2024年7月1日七月 上午2:35，maobibo写道：
[...]
>
> How about split loongson_ipi.c into 
> loongson_ipi_base.c/loongson_ipi_loongson.c/loongson_ipi_loongarch.c,
>
> File loongson_ipi_base.c contains the common code, loongson_ipi_xxx.c 
> contains arch specific. Soon we will submit irqchip in kernel function,
> it will be much different for architectures, since ioctl command is 
> different for two architectures to save/restore ipi registers.

MIPS's in kernel IPI IOCTL interface is non-existent so far, so if you are going
to design something, I think it will be adopted by MIPS if necessary. There is still
no need to create divergence in between.

That being said, You are more than welcomed to draft a patch so we can discuss based
on that.

Thanks
>
> Regards
> Bibo Mao

-- 
- Jiaxun

Re: [PATCH v3 1/4] hw/intc: Remove loongarch_ipi.c

[maobibo]

On 2024/7/1 下午3:08, Jiaxun Yang wrote:
> 
> 
> 在2024年7月1日七月 上午2:35，maobibo写道：
> [...]
>>
>> How about split loongson_ipi.c into
>> loongson_ipi_base.c/loongson_ipi_loongson.c/loongson_ipi_loongarch.c,
>>
>> File loongson_ipi_base.c contains the common code, loongson_ipi_xxx.c
>> contains arch specific. Soon we will submit irqchip in kernel function,
>> it will be much different for architectures, since ioctl command is
>> different for two architectures to save/restore ipi registers.
> 
> MIPS's in kernel IPI IOCTL interface is non-existent so far, so if you are going
> to design something, I think it will be adopted by MIPS if necessary. There is still
> no need to create divergence in between.
> 
> That being said, You are more than welcomed to draft a patch so we can discuss based
> on that.
Sure, will do.

Regards
Bibo Mao
> 
> Thanks
>>
>> Regards
>> Bibo Mao
> 

[PATCH v3 2/4] hw/intc/loongson_ipi: Provide per core MMIO address spaces

[Jiaxun Yang]

The real IPI hardware have dedicated MMIO registers mapped into
memory address space for every core. This is not used by LoongArch
guest software but it is essential for CPU without IOCSR such as
Loongson-3A1000.

Implement it with existing infrastructure.

Acked-by: Song Gao <gaosong@loongson.cn>
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/intc/loongson_ipi.c         | 81 ++++++++++++++++++++++++++++++++----------
 include/hw/intc/loongson_ipi.h |  2 ++
 2 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/hw/intc/loongson_ipi.c b/hw/intc/loongson_ipi.c
index 93cc50a37a11..c8a25b4eb8e2 100644
--- a/hw/intc/loongson_ipi.c
+++ b/hw/intc/loongson_ipi.c
@@ -23,16 +23,14 @@
 #endif
 #include "trace.h"
 
-static MemTxResult loongson_ipi_readl(void *opaque, hwaddr addr,
+static MemTxResult loongson_ipi_core_readl(void *opaque, hwaddr addr,
                                        uint64_t *data,
                                        unsigned size, MemTxAttrs attrs)
 {
-    IPICore *s;
-    LoongsonIPI *ipi = opaque;
+    IPICore *s = opaque;
     uint64_t ret = 0;
     int index = 0;
 
-    s = &ipi->cpu[attrs.requester_id];
     addr &= 0xff;
     switch (addr) {
     case CORE_STATUS_OFF:
@@ -61,6 +59,21 @@ static MemTxResult loongson_ipi_readl(void *opaque, hwaddr addr,
     return MEMTX_OK;
 }
 
+static MemTxResult loongson_ipi_iocsr_readl(void *opaque, hwaddr addr,
+                                       uint64_t *data,
+                                       unsigned size, MemTxAttrs attrs)
+{
+    LoongsonIPI *ipi = opaque;
+    IPICore *s;
+
+    if (attrs.requester_id >= ipi->num_cpu) {
+        return MEMTX_DECODE_ERROR;
+    }
+
+    s = &ipi->cpu[attrs.requester_id];
+    return loongson_ipi_core_readl(s, addr, data, size, attrs);
+}
+
 static AddressSpace *get_cpu_iocsr_as(CPUState *cpu)
 {
 #ifdef TARGET_LOONGARCH64
@@ -174,17 +187,17 @@ static MemTxResult any_send(uint64_t val, MemTxAttrs attrs)
     return send_ipi_data(cs, val, addr, attrs);
 }
 
-static MemTxResult loongson_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
-                                        unsigned size, MemTxAttrs attrs)
+static MemTxResult loongson_ipi_core_writel(void *opaque, hwaddr addr,
+                                            uint64_t val, unsigned size,
+                                            MemTxAttrs attrs)
 {
-    LoongsonIPI *ipi = opaque;
-    IPICore *s;
+    IPICore *s = opaque;
+    LoongsonIPI *ipi = s->ipi;
     int index = 0;
     uint32_t cpuid;
     uint8_t vector;
     CPUState *cs;
 
-    s = &ipi->cpu[attrs.requester_id];
     addr &= 0xff;
     trace_loongson_ipi_write(size, (uint64_t)addr, val);
     switch (addr) {
@@ -215,13 +228,11 @@ static MemTxResult loongson_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
         /* IPI status vector */
         vector = extract8(val, 0, 5);
         cs = ipi_getcpu(cpuid);
-        if (cs == NULL) {
+        if (cs == NULL || cs->cpu_index >= ipi->num_cpu) {
             return MEMTX_DECODE_ERROR;
         }
-
-        /* override requester_id */
-        attrs.requester_id = cs->cpu_index;
-        loongson_ipi_writel(ipi, CORE_SET_OFF, BIT(vector), 4, attrs);
+        loongson_ipi_core_writel(&ipi->cpu[cs->cpu_index], CORE_SET_OFF,
+                                 BIT(vector), 4, attrs);
         break;
     default:
         qemu_log_mask(LOG_UNIMP, "invalid write: %x", (uint32_t)addr);
@@ -231,9 +242,34 @@ static MemTxResult loongson_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
     return MEMTX_OK;
 }
 
-static const MemoryRegionOps loongson_ipi_ops = {
-    .read_with_attrs = loongson_ipi_readl,
-    .write_with_attrs = loongson_ipi_writel,
+static MemTxResult loongson_ipi_iocsr_writel(void *opaque, hwaddr addr,
+                                            uint64_t val, unsigned size,
+                                            MemTxAttrs attrs)
+{
+    LoongsonIPI *ipi = opaque;
+    IPICore *s;
+
+    if (attrs.requester_id >= ipi->num_cpu) {
+        return MEMTX_DECODE_ERROR;
+    }
+
+    s = &ipi->cpu[attrs.requester_id];
+    return loongson_ipi_core_writel(s, addr, val, size, attrs);
+}
+
+static const MemoryRegionOps loongson_ipi_core_ops = {
+    .read_with_attrs = loongson_ipi_core_readl,
+    .write_with_attrs = loongson_ipi_core_writel,
+    .impl.min_access_size = 4,
+    .impl.max_access_size = 4,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 8,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static const MemoryRegionOps loongson_ipi_iocsr_ops = {
+    .read_with_attrs = loongson_ipi_iocsr_readl,
+    .write_with_attrs = loongson_ipi_iocsr_writel,
     .impl.min_access_size = 4,
     .impl.max_access_size = 4,
     .valid.min_access_size = 4,
@@ -282,7 +318,7 @@ static void loongson_ipi_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), &loongson_ipi_ops,
+    memory_region_init_io(&s->ipi_iocsr_mem, OBJECT(dev), &loongson_ipi_iocsr_ops,
                           s, "loongson_ipi_iocsr", 0x48);
 
     /* loongson_ipi_iocsr performs re-entrant IO through ipi_send */
@@ -297,11 +333,18 @@ static void loongson_ipi_realize(DeviceState *dev, Error **errp)
 
     s->cpu = g_new0(IPICore, s->num_cpu);
     if (s->cpu == NULL) {
-        error_setg(errp, "Memory allocation for ExtIOICore faile");
+        error_setg(errp, "Memory allocation for IPICore faile");
         return;
     }
 
     for (i = 0; i < s->num_cpu; i++) {
+        s->cpu[i].ipi = s;
+        s->cpu[i].ipi_mmio_mem = g_new0(MemoryRegion, 1);
+        g_autofree char *name = g_strdup_printf("loongson_ipi_cpu%d_mmio", i);
+        memory_region_init_io(s->cpu[i].ipi_mmio_mem, OBJECT(dev),
+                              &loongson_ipi_core_ops, &s->cpu[i], name, 0x48);
+        sysbus_init_mmio(sbd, s->cpu[i].ipi_mmio_mem);
+
         qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
     }
 }
diff --git a/include/hw/intc/loongson_ipi.h b/include/hw/intc/loongson_ipi.h
index 2c0e8820f5ec..3f795edbf3cd 100644
--- a/include/hw/intc/loongson_ipi.h
+++ b/include/hw/intc/loongson_ipi.h
@@ -34,6 +34,8 @@
 OBJECT_DECLARE_SIMPLE_TYPE(LoongsonIPI, LOONGSON_IPI)
 
 typedef struct IPICore {
+    LoongsonIPI *ipi;
+    MemoryRegion *ipi_mmio_mem;
     uint32_t status;
     uint32_t en;
     uint32_t set;

-- 
2.43.0

[PATCH v3 3/4] hw/mips/loongson3_virt: Wire up loongson_ipi device

[Jiaxun Yang]

Wire up loongson_ipi device for loongson3_virt machine, so we
can have SMP support for TCG backend as well.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/mips/Kconfig           |  1 +
 hw/mips/loongson3_bootp.c |  2 --
 hw/mips/loongson3_bootp.h |  3 +++
 hw/mips/loongson3_virt.c  | 39 +++++++++++++++++++++++++++++++++++++--
 4 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index a7f26edebe84..692bede538ee 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -67,6 +67,7 @@ config LOONGSON3V
     imply USB_OHCI_PCI
     select SERIAL
     select GOLDFISH_RTC
+    select LOONGSON_IPI
     select LOONGSON_LIOINTC
     select PCI_EXPRESS_GENERIC_BRIDGE
     select MSI_NONBROKEN
diff --git a/hw/mips/loongson3_bootp.c b/hw/mips/loongson3_bootp.c
index 03a10b63c1b4..b97b81903b74 100644
--- a/hw/mips/loongson3_bootp.c
+++ b/hw/mips/loongson3_bootp.c
@@ -25,8 +25,6 @@
 #include "hw/boards.h"
 #include "hw/mips/loongson3_bootp.h"
 
-#define LOONGSON3_CORE_PER_NODE 4
-
 static void init_cpu_info(void *g_cpuinfo, uint64_t cpu_freq)
 {
     struct efi_cpuinfo_loongson *c = g_cpuinfo;
diff --git a/hw/mips/loongson3_bootp.h b/hw/mips/loongson3_bootp.h
index 1b0dd3b59171..9091265df7fc 100644
--- a/hw/mips/loongson3_bootp.h
+++ b/hw/mips/loongson3_bootp.h
@@ -200,6 +200,8 @@ struct boot_params {
     struct efi_reset_system_t reset_system;
 };
 
+#define LOONGSON3_CORE_PER_NODE 4
+
 /* Overall MMIO & Memory layout */
 enum {
     VIRT_LOWMEM,
@@ -211,6 +213,7 @@ enum {
     VIRT_BIOS_ROM,
     VIRT_UART,
     VIRT_LIOINTC,
+    VIRT_IPI,
     VIRT_PCIE_MMIO,
     VIRT_HIGHMEM
 };
diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 440268a07430..3a4105228c1d 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -36,6 +36,7 @@
 #include "hw/mips/loongson3_bootp.h"
 #include "hw/misc/unimp.h"
 #include "hw/intc/i8259.h"
+#include "hw/intc/loongson_ipi.h"
 #include "hw/loader.h"
 #include "hw/isa/superio.h"
 #include "hw/pci/msi.h"
@@ -74,6 +75,7 @@ const MemMapEntry virt_memmap[] = {
     [VIRT_PCIE_ECAM] =   { 0x1a000000,     0x2000000 },
     [VIRT_BIOS_ROM] =    { 0x1fc00000,      0x200000 },
     [VIRT_UART] =        { 0x1fe001e0,           0x8 },
+    [VIRT_IPI] =         { 0x3ff01000,         0x400 },
     [VIRT_LIOINTC] =     { 0x3ff01400,          0x64 },
     [VIRT_PCIE_MMIO] =   { 0x40000000,    0x40000000 },
     [VIRT_HIGHMEM] =     { 0x80000000,           0x0 }, /* Variable */
@@ -485,6 +487,7 @@ static void mips_loongson3_virt_init(MachineState *machine)
     Clock *cpuclk;
     CPUMIPSState *env;
     DeviceState *liointc;
+    DeviceState *ipi = NULL;
     char *filename;
     const char *kernel_cmdline = machine->kernel_cmdline;
     const char *kernel_filename = machine->kernel_filename;
@@ -494,6 +497,7 @@ static void mips_loongson3_virt_init(MachineState *machine)
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     MemoryRegion *iomem = g_new(MemoryRegion, 1);
+    MemoryRegion *iocsr = g_new(MemoryRegion, 1);
 
     /* TODO: TCG will support all CPU types */
     if (!kvm_enabled()) {
@@ -527,6 +531,19 @@ static void mips_loongson3_virt_init(MachineState *machine)
     create_unimplemented_device("mmio fallback 0", 0x10000000, 256 * MiB);
     create_unimplemented_device("mmio fallback 1", 0x30000000, 256 * MiB);
 
+    memory_region_init(iocsr, OBJECT(machine), "loongson3.iocsr", UINT32_MAX);
+
+    /* IPI controller is in kernel for KVM */
+    if (!kvm_enabled()) {
+        ipi = qdev_new(TYPE_LOONGSON_IPI);
+        qdev_prop_set_uint32(ipi, "num-cpu", machine->smp.cpus);
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
+        memory_region_add_subregion(iocsr, SMP_IPI_MAILBOX,
+            sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi), 0));
+        memory_region_add_subregion(iocsr, MAIL_SEND_ADDR,
+            sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi), 1));
+    }
+
     liointc = qdev_new("loongson.liointc");
     sysbus_realize_and_unref(SYS_BUS_DEVICE(liointc), &error_fatal);
 
@@ -543,6 +560,8 @@ static void mips_loongson3_virt_init(MachineState *machine)
     clock_set_hz(cpuclk, DEF_LOONGSON3_FREQ);
 
     for (i = 0; i < machine->smp.cpus; i++) {
+        int node = i / LOONGSON3_CORE_PER_NODE;
+        int core = i % LOONGSON3_CORE_PER_NODE;
         int ip;
 
         /* init CPUs */
@@ -553,12 +572,28 @@ static void mips_loongson3_virt_init(MachineState *machine)
         cpu_mips_clock_init(cpu);
         qemu_register_reset(main_cpu_reset, cpu);
 
-        if (i >= 4) {
+        if (ipi) {
+            hwaddr base = ((hwaddr)node << 44) + virt_memmap[VIRT_IPI].base;
+            base += core * 0x100;
+            qdev_connect_gpio_out(ipi, i, cpu->env.irq[6]);
+            sysbus_mmio_map(SYS_BUS_DEVICE(ipi), i + 2, base);
+        }
+
+        if (ase_lcsr_available(&MIPS_CPU(cpu)->env)) {
+            MemoryRegion *core_iocsr = g_new(MemoryRegion, 1);
+            g_autofree char *name = g_strdup_printf("loongson3.core%d_iocsr", i);
+            memory_region_init_alias(core_iocsr, OBJECT(cpu), name,
+                                     iocsr, 0, UINT32_MAX);
+            memory_region_add_subregion(&MIPS_CPU(cpu)->env.iocsr.mr,
+                                        0, core_iocsr);
+        }
+
+        if (node > 0) {
             continue; /* Only node-0 can be connected to LIOINTC */
         }
 
         for (ip = 0; ip < 4 ; ip++) {
-            int pin = i * 4 + ip;
+            int pin = core * LOONGSON3_CORE_PER_NODE + ip;
             sysbus_connect_irq(SYS_BUS_DEVICE(liointc),
                                pin, cpu->env.irq[ip + 2]);
         }

-- 
2.43.0

Re: [PATCH v3 3/4] hw/mips/loongson3_virt: Wire up loongson_ipi device

[Philippe Mathieu-Daudé]

On 5/6/24 04:15, Jiaxun Yang wrote:
> Wire up loongson_ipi device for loongson3_virt machine, so we
> can have SMP support for TCG backend as well.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   hw/mips/Kconfig           |  1 +
>   hw/mips/loongson3_bootp.c |  2 --
>   hw/mips/loongson3_bootp.h |  3 +++
>   hw/mips/loongson3_virt.c  | 39 +++++++++++++++++++++++++++++++++++++--
>   4 files changed, 41 insertions(+), 4 deletions(-)


> @@ -527,6 +531,19 @@ static void mips_loongson3_virt_init(MachineState *machine)
>       create_unimplemented_device("mmio fallback 0", 0x10000000, 256 * MiB);
>       create_unimplemented_device("mmio fallback 1", 0x30000000, 256 * MiB);
>   
> +    memory_region_init(iocsr, OBJECT(machine), "loongson3.iocsr", UINT32_MAX);
> +
> +    /* IPI controller is in kernel for KVM */
> +    if (!kvm_enabled()) {

Generically one could come with another hypervisor support, so better
to check for what you are expecting. You could see some uses of:

   if (tcg) ... else if (kvm) ... else abort().

> +        ipi = qdev_new(TYPE_LOONGSON_IPI);
> +        qdev_prop_set_uint32(ipi, "num-cpu", machine->smp.cpus);
> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
> +        memory_region_add_subregion(iocsr, SMP_IPI_MAILBOX,
> +            sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi), 0));
> +        memory_region_add_subregion(iocsr, MAIL_SEND_ADDR,
> +            sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi), 1));
> +    }
> +
>       liointc = qdev_new("loongson.liointc");
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(liointc), &error_fatal);
>   
> @@ -543,6 +560,8 @@ static void mips_loongson3_virt_init(MachineState *machine)
>       clock_set_hz(cpuclk, DEF_LOONGSON3_FREQ);
>   
>       for (i = 0; i < machine->smp.cpus; i++) {
> +        int node = i / LOONGSON3_CORE_PER_NODE;
> +        int core = i % LOONGSON3_CORE_PER_NODE;
>           int ip;
>   
>           /* init CPUs */
> @@ -553,12 +572,28 @@ static void mips_loongson3_virt_init(MachineState *machine)
>           cpu_mips_clock_init(cpu);
>           qemu_register_reset(main_cpu_reset, cpu);
>   
> -        if (i >= 4) {
> +        if (ipi) {
> +            hwaddr base = ((hwaddr)node << 44) + virt_memmap[VIRT_IPI].base;
> +            base += core * 0x100;
> +            qdev_connect_gpio_out(ipi, i, cpu->env.irq[6]);
> +            sysbus_mmio_map(SYS_BUS_DEVICE(ipi), i + 2, base);
> +        }
> +
> +        if (ase_lcsr_available(&MIPS_CPU(cpu)->env)) {
> +            MemoryRegion *core_iocsr = g_new(MemoryRegion, 1);
> +            g_autofree char *name = g_strdup_printf("loongson3.core%d_iocsr", i);
> +            memory_region_init_alias(core_iocsr, OBJECT(cpu), name,
> +                                     iocsr, 0, UINT32_MAX);
> +            memory_region_add_subregion(&MIPS_CPU(cpu)->env.iocsr.mr,
> +                                        0, core_iocsr);
> +        }
> +
> +        if (node > 0) {
>               continue; /* Only node-0 can be connected to LIOINTC */
>           }

Pre-existing, but the logic appears clearer as:

           if (node == 0) {
>   
>           for (ip = 0; ip < 4 ; ip++) {
> -            int pin = i * 4 + ip;
> +            int pin = core * LOONGSON3_CORE_PER_NODE + ip;
>               sysbus_connect_irq(SYS_BUS_DEVICE(liointc),
>                                  pin, cpu->env.irq[ip + 2]);
>           }
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 3/4] hw/mips/loongson3_virt: Wire up loongson_ipi device

[Philippe Mathieu-Daudé]

On 18/6/24 13:17, Philippe Mathieu-Daudé wrote:
> On 5/6/24 04:15, Jiaxun Yang wrote:
>> Wire up loongson_ipi device for loongson3_virt machine, so we
>> can have SMP support for TCG backend as well.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>   hw/mips/Kconfig           |  1 +
>>   hw/mips/loongson3_bootp.c |  2 --
>>   hw/mips/loongson3_bootp.h |  3 +++
>>   hw/mips/loongson3_virt.c  | 39 +++++++++++++++++++++++++++++++++++++--
>>   4 files changed, 41 insertions(+), 4 deletions(-)
> 
> 
>> @@ -527,6 +531,19 @@ static void mips_loongson3_virt_init(MachineState 
>> *machine)
>>       create_unimplemented_device("mmio fallback 0", 0x10000000, 256 * 
>> MiB);
>>       create_unimplemented_device("mmio fallback 1", 0x30000000, 256 * 
>> MiB);
>> +    memory_region_init(iocsr, OBJECT(machine), "loongson3.iocsr", 
>> UINT32_MAX);
>> +
>> +    /* IPI controller is in kernel for KVM */
>> +    if (!kvm_enabled()) {
> 
> Generically one could come with another hypervisor support, so better
> to check for what you are expecting. You could see some uses of:
> 
>    if (tcg) ... else if (kvm) ... else abort().
> 
>> +        ipi = qdev_new(TYPE_LOONGSON_IPI);
>> +        qdev_prop_set_uint32(ipi, "num-cpu", machine->smp.cpus);
>> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
>> +        memory_region_add_subregion(iocsr, SMP_IPI_MAILBOX,
>> +            sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi), 0));
>> +        memory_region_add_subregion(iocsr, MAIL_SEND_ADDR,
>> +            sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi), 1));
>> +    }
>> +
>>       liointc = qdev_new("loongson.liointc");
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(liointc), &error_fatal);
>> @@ -543,6 +560,8 @@ static void mips_loongson3_virt_init(MachineState 
>> *machine)
>>       clock_set_hz(cpuclk, DEF_LOONGSON3_FREQ);
>>       for (i = 0; i < machine->smp.cpus; i++) {
>> +        int node = i / LOONGSON3_CORE_PER_NODE;
>> +        int core = i % LOONGSON3_CORE_PER_NODE;
>>           int ip;
>>           /* init CPUs */
>> @@ -553,12 +572,28 @@ static void 
>> mips_loongson3_virt_init(MachineState *machine)
>>           cpu_mips_clock_init(cpu);
>>           qemu_register_reset(main_cpu_reset, cpu);
>> -        if (i >= 4) {
>> +        if (ipi) {
>> +            hwaddr base = ((hwaddr)node << 44) + 
>> virt_memmap[VIRT_IPI].base;
>> +            base += core * 0x100;
>> +            qdev_connect_gpio_out(ipi, i, cpu->env.irq[6]);
>> +            sysbus_mmio_map(SYS_BUS_DEVICE(ipi), i + 2, base);
>> +        }
>> +
>> +        if (ase_lcsr_available(&MIPS_CPU(cpu)->env)) {
>> +            MemoryRegion *core_iocsr = g_new(MemoryRegion, 1);

Please store this in LoongsonMachineState (new patch):

   MemoryRegion core_iocsr[LOONGSON_MAX_VCPUS];

>> +            g_autofree char *name = 
>> g_strdup_printf("loongson3.core%d_iocsr", i);
>> +            memory_region_init_alias(core_iocsr, OBJECT(cpu), name,
>> +                                     iocsr, 0, UINT32_MAX);
>> +            memory_region_add_subregion(&MIPS_CPU(cpu)->env.iocsr.mr,
>> +                                        0, core_iocsr);
>> +        }
>> +
>> +        if (node > 0) {
>>               continue; /* Only node-0 can be connected to LIOINTC */
>>           }
> 
> Pre-existing, but the logic appears clearer as:
> 
>            if (node == 0) {
>>           for (ip = 0; ip < 4 ; ip++) {
>> -            int pin = i * 4 + ip;
>> +            int pin = core * LOONGSON3_CORE_PER_NODE + ip;
>>               sysbus_connect_irq(SYS_BUS_DEVICE(liointc),
>>                                  pin, cpu->env.irq[ip + 2]);
>>           }
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

[PATCH v3 4/4] hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id

[Jiaxun Yang]

cpu_by_arch_id is doing the same thing as our ipi_getcpu logic.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/intc/loongson_ipi.c | 39 +++------------------------------------
 1 file changed, 3 insertions(+), 36 deletions(-)

diff --git a/hw/intc/loongson_ipi.c b/hw/intc/loongson_ipi.c
index c8a25b4eb8e2..b3cda5972793 100644
--- a/hw/intc/loongson_ipi.c
+++ b/hw/intc/loongson_ipi.c
@@ -118,39 +118,6 @@ static MemTxResult send_ipi_data(CPUState *cpu, uint64_t val, hwaddr addr,
     return MEMTX_OK;
 }
 
-static int archid_cmp(const void *a, const void *b)
-{
-   CPUArchId *archid_a = (CPUArchId *)a;
-   CPUArchId *archid_b = (CPUArchId *)b;
-
-   return archid_a->arch_id - archid_b->arch_id;
-}
-
-static CPUArchId *find_cpu_by_archid(MachineState *ms, uint32_t id)
-{
-    CPUArchId apic_id, *found_cpu;
-
-    apic_id.arch_id = id;
-    found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
-        ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
-        archid_cmp);
-
-    return found_cpu;
-}
-
-static CPUState *ipi_getcpu(int arch_id)
-{
-    MachineState *machine = MACHINE(qdev_get_machine());
-    CPUArchId *archid;
-
-    archid = find_cpu_by_archid(machine, arch_id);
-    if (archid) {
-        return CPU(archid->cpu);
-    }
-
-    return NULL;
-}
-
 static MemTxResult mail_send(uint64_t val, MemTxAttrs attrs)
 {
     uint32_t cpuid;
@@ -158,7 +125,7 @@ static MemTxResult mail_send(uint64_t val, MemTxAttrs attrs)
     CPUState *cs;
 
     cpuid = extract32(val, 16, 10);
-    cs = ipi_getcpu(cpuid);
+    cs = cpu_by_arch_id(cpuid);
     if (cs == NULL) {
         return MEMTX_DECODE_ERROR;
     }
@@ -176,7 +143,7 @@ static MemTxResult any_send(uint64_t val, MemTxAttrs attrs)
     CPUState *cs;
 
     cpuid = extract32(val, 16, 10);
-    cs = ipi_getcpu(cpuid);
+    cs = cpu_by_arch_id(cpuid);
     if (cs == NULL) {
         return MEMTX_DECODE_ERROR;
     }
@@ -227,7 +194,7 @@ static MemTxResult loongson_ipi_core_writel(void *opaque, hwaddr addr,
         cpuid = extract32(val, 16, 10);
         /* IPI status vector */
         vector = extract8(val, 0, 5);
-        cs = ipi_getcpu(cpuid);
+        cs = cpu_by_arch_id(cpuid);
         if (cs == NULL || cs->cpu_index >= ipi->num_cpu) {
             return MEMTX_DECODE_ERROR;
         }

-- 
2.43.0

Re: [PATCH v3 4/4] hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id

[gaosong]

在 2024/6/5 上午10:15, Jiaxun Yang 写道:
> cpu_by_arch_id is doing the same thing as our ipi_getcpu logic.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
Reviewed-by: Song Gao <gaosong@loongson.cn>

Thanks.
Song Gao
>   hw/intc/loongson_ipi.c | 39 +++------------------------------------
>   1 file changed, 3 insertions(+), 36 deletions(-)
>
> diff --git a/hw/intc/loongson_ipi.c b/hw/intc/loongson_ipi.c
> index c8a25b4eb8e2..b3cda5972793 100644
> --- a/hw/intc/loongson_ipi.c
> +++ b/hw/intc/loongson_ipi.c
> @@ -118,39 +118,6 @@ static MemTxResult send_ipi_data(CPUState *cpu, uint64_t val, hwaddr addr,
>       return MEMTX_OK;
>   }
>   
> -static int archid_cmp(const void *a, const void *b)
> -{
> -   CPUArchId *archid_a = (CPUArchId *)a;
> -   CPUArchId *archid_b = (CPUArchId *)b;
> -
> -   return archid_a->arch_id - archid_b->arch_id;
> -}
> -
> -static CPUArchId *find_cpu_by_archid(MachineState *ms, uint32_t id)
> -{
> -    CPUArchId apic_id, *found_cpu;
> -
> -    apic_id.arch_id = id;
> -    found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
> -        ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
> -        archid_cmp);
> -
> -    return found_cpu;
> -}
> -
> -static CPUState *ipi_getcpu(int arch_id)
> -{
> -    MachineState *machine = MACHINE(qdev_get_machine());
> -    CPUArchId *archid;
> -
> -    archid = find_cpu_by_archid(machine, arch_id);
> -    if (archid) {
> -        return CPU(archid->cpu);
> -    }
> -
> -    return NULL;
> -}
> -
>   static MemTxResult mail_send(uint64_t val, MemTxAttrs attrs)
>   {
>       uint32_t cpuid;
> @@ -158,7 +125,7 @@ static MemTxResult mail_send(uint64_t val, MemTxAttrs attrs)
>       CPUState *cs;
>   
>       cpuid = extract32(val, 16, 10);
> -    cs = ipi_getcpu(cpuid);
> +    cs = cpu_by_arch_id(cpuid);
>       if (cs == NULL) {
>           return MEMTX_DECODE_ERROR;
>       }
> @@ -176,7 +143,7 @@ static MemTxResult any_send(uint64_t val, MemTxAttrs attrs)
>       CPUState *cs;
>   
>       cpuid = extract32(val, 16, 10);
> -    cs = ipi_getcpu(cpuid);
> +    cs = cpu_by_arch_id(cpuid);
>       if (cs == NULL) {
>           return MEMTX_DECODE_ERROR;
>       }
> @@ -227,7 +194,7 @@ static MemTxResult loongson_ipi_core_writel(void *opaque, hwaddr addr,
>           cpuid = extract32(val, 16, 10);
>           /* IPI status vector */
>           vector = extract8(val, 0, 5);
> -        cs = ipi_getcpu(cpuid);
> +        cs = cpu_by_arch_id(cpuid);
>           if (cs == NULL || cs->cpu_index >= ipi->num_cpu) {
>               return MEMTX_DECODE_ERROR;
>           }
>

Re: [PATCH v3 0/4] hw/mips/loongson3_virt: Implement IPI support

[Jiaxun Yang]

在2024年6月5日六月 上午3:15，Jiaxun Yang写道：
> Hi all,
>
> This series enabled IPI support for loongson3 virt board, loosely
> based on my previous work[1].
> It generalized loongarch_ipi device to share among both loongarch
> and MIPS machines. 
>
> Thanks

A genttle ping :-)

Thanks
- Jiaxun

>
> [1]: https://lore.kernel.org/all/20230521102307.87081-1-jiaxun.yang@flygoat.com/
>
> To: qemu-devel@nongnu.org
> Cc: Song Gao <gaosong@loongson.cn>
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> Changes in v3:
> - Fix up sign off lines.
> - Link to v2: 
> https://lore.kernel.org/r/20240605-loongson3-ipi-v2-0-da26282e03a5@flygoat.com
>
> Changes in v2:
> - Add a patch to remove loongarch_ipi.c (Gao)
> - Change iocsr alias owner to CPU (philmd)
> - New patch to remove ipi_getcpu
> - Link to v1: 
> https://lore.kernel.org/r/20240508-loongson3-ipi-v1-0-1a7b67704664@flygoat.com
>
> ---
> Jiaxun Yang (4):
>       hw/intc: Remove loongarch_ipi.c
>       hw/intc/loongson_ipi: Provide per core MMIO address spaces
>       hw/mips/loongson3_virt: Wire up loongson_ipi device
>       hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id
>
>  hw/intc/loongarch_ipi.c        | 347 -----------------------------------------
>  hw/intc/loongson_ipi.c         | 120 +++++++-------
>  hw/mips/Kconfig                |   1 +
>  hw/mips/loongson3_bootp.c      |   2 -
>  hw/mips/loongson3_bootp.h      |   3 +
>  hw/mips/loongson3_virt.c       |  39 ++++-
>  include/hw/intc/loongson_ipi.h |   2 +
>  7 files changed, 108 insertions(+), 406 deletions(-)
> ---
> base-commit: d67a6e054b92e5e1cbb7b0bd5782a670cc7f0df7
> change-id: 20240508-loongson3-ipi-65e99b47c400
>
> Best regards,
> -- 
> Jiaxun Yang <jiaxun.yang@flygoat.com>

-- 
- Jiaxun

Re: [PATCH v3 0/4] hw/mips/loongson3_virt: Implement IPI support

[Philippe Mathieu-Daudé]

On 12/6/24 14:09, Jiaxun Yang wrote:
> 在2024年6月5日六月 上午3:15，Jiaxun Yang写道：
>> Hi all,
>>
>> This series enabled IPI support for loongson3 virt board, loosely
>> based on my previous work[1].
>> It generalized loongarch_ipi device to share among both loongarch
>> and MIPS machines.
>>
>> Thanks
> 
> A genttle ping :-)

Series queued, thanks!

>> ---
>> Jiaxun Yang (4):
>>        hw/intc: Remove loongarch_ipi.c
>>        hw/intc/loongson_ipi: Provide per core MMIO address spaces
>>        hw/mips/loongson3_virt: Wire up loongson_ipi device
>>        hw/intc/loongson_ipi: Replace ipi_getcpu with cpu_by_arch_id