Re: [PATCH v2 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[Richard Henderson]

On 4/25/22 02:10, Xiaojuan Yang wrote:
> This patch realize the EIOINTC interrupt controller.
>
> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   hw/intc/Kconfig                    |   3 +
>   hw/intc/loongarch_extioi.c         | 483 +++++++++++++++++++++++++++++
>   hw/intc/meson.build                |   1 +
>   hw/intc/trace-events               |   9 +
>   hw/loongarch/Kconfig               |   1 +
>   include/hw/intc/loongarch_extioi.h |  60 ++++
>   6 files changed, 557 insertions(+)
>   create mode 100644 hw/intc/loongarch_extioi.c
>   create mode 100644 include/hw/intc/loongarch_extioi.h
>
> diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
> index 71c04c328e..28bd1f185d 100644
> --- a/hw/intc/Kconfig
> +++ b/hw/intc/Kconfig
> @@ -96,3 +96,6 @@ config LOONGARCH_PCH_MSI
>       select MSI_NONBROKEN
>       bool
>       select UNIMP
> +
> +config LOONGARCH_EXTIOI
> +    bool
> diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
> new file mode 100644
> index 0000000000..1d9317c5bd
> --- /dev/null
> +++ b/hw/intc/loongarch_extioi.c
> @@ -0,0 +1,483 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Loongson 3A5000 ext interrupt controller emulation
> + *
> + * Copyright (C) 2021 Loongson Technology Corporation Limited
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qemu/log.h"
> +#include "hw/irq.h"
> +#include "hw/sysbus.h"
> +#include "hw/loongarch/virt.h"
> +#include "hw/qdev-properties.h"
> +#include "exec/address-spaces.h"
> +#include "hw/intc/loongarch_extioi.h"
> +#include "migration/vmstate.h"
> +#include "trace.h"
> +
> +static void extioi_update_irq(LoongArchExtIOI *s, int irq_num, int level)
> +{
> +    uint64_t ipnum, cpu;
> +
> +    /*
> +     * Routing in group of 32 interrupts.
> +     * The default value of csr[0x420][49]
> +     * is 0 and nobody will change it,
> +     * so 'ipmap' use bitmap function.
> +     */
> +    ipnum = ldub_p((void *)&s->ipmap + (irq_num / 32)) & 0xf;
> +    ipnum = find_first_bit(&ipnum, 4);
> +    ipnum = (ipnum == 4) ? 0 : ipnum;
> +
> +    cpu = ldub_p((void *)s->coremap + irq_num) & 0xf;
> +    cpu = find_first_bit(&cpu, 4);
> +    cpu = (cpu == 4) ? 0 : cpu;
> +
> +    if (level) {
> +        if (test_bit(irq_num, (unsigned long *)s->enable) == false) {
> +            return;
> +        }
> +        bitmap_set((unsigned long *)s->coreisr[cpu], irq_num, 1);
> +        qemu_set_irq(s->parent_irq[cpu][ipnum], level);
> +    } else {
> +        bitmap_clear((unsigned long *)s->coreisr[cpu], irq_num, 1);
> +        qemu_set_irq(s->parent_irq[cpu][ipnum], level);

I told you before: all of this casting is wrong.
You cannot use only part of the bitmap.h interface.

> +#define MAX_CORES                  LOONGARCH_MAX_VCPUS

Why do you have so many MAX cpu defines, all different?


r~