Re: [PATCH 1/4] hw/intc/loongarch_ipi: Bring back all 4 IPI mailboxes

[Peter Maydell]

On Sun, 21 May 2023 at 11:24, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> As per "Loongson 3A5000/3B5000 Processor Reference Manual",
> Loongson 3A5000's IPI implementation have 4 mailboxes per
> core.
>
> However, in 78464f023b54 ("hw/loongarch/virt: Modify ipi as
> percpu device"), the number of IPI mailboxes was reduced to
> one, which mismatches actual hardware.
>
> It won't affect LoongArch based system as LoongArch boot code
> only uses the first mailbox, however MIPS based Loongson boot
> code uses all 4 mailboxes.
>
> Fixes: 78464f023b54 ("hw/loongarch/virt: Modify ipi as percpu device")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/intc/loongarch_ipi.c         | 6 +++---
>  include/hw/intc/loongarch_ipi.h | 4 +++-
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
> index d6ab91721ea1..3e453816524e 100644
> --- a/hw/intc/loongarch_ipi.c
> +++ b/hw/intc/loongarch_ipi.c
> @@ -238,14 +238,14 @@ static void loongarch_ipi_init(Object *obj)
>
>  static const VMStateDescription vmstate_ipi_core = {
>      .name = "ipi-single",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(status, IPICore),
>          VMSTATE_UINT32(en, IPICore),
>          VMSTATE_UINT32(set, IPICore),
>          VMSTATE_UINT32(clear, IPICore),
> -        VMSTATE_UINT32_ARRAY(buf, IPICore, 2),
> +        VMSTATE_UINT32_ARRAY(buf, IPICore, IPI_MBX_NUM * 2),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> diff --git a/include/hw/intc/loongarch_ipi.h b/include/hw/intc/loongarch_ipi.h
> index 664e050b926e..6c6194786e80 100644
> --- a/include/hw/intc/loongarch_ipi.h
> +++ b/include/hw/intc/loongarch_ipi.h
> @@ -28,6 +28,8 @@
>  #define MAIL_SEND_OFFSET      0
>  #define ANY_SEND_OFFSET       (IOCSR_ANY_SEND - IOCSR_MAIL_SEND)
>
> +#define IPI_MBX_NUM           4
> +
>  #define TYPE_LOONGARCH_IPI "loongarch_ipi"
>  OBJECT_DECLARE_SIMPLE_TYPE(LoongArchIPI, LOONGARCH_IPI)
>
> @@ -37,7 +39,7 @@ typedef struct IPICore {
>      uint32_t set;
>      uint32_t clear;
>      /* 64bit buf divide into 2 32bit buf */
> -    uint32_t buf[2];
> +    uint32_t buf[IPI_MBX_NUM * 2];
>      qemu_irq irq;
>  } IPICore;

In particular, this fixes Coverity issues CID 1512452 and 1512453,
where Coverity notices that the code in loongarch_ipi_writel() and
loongarch_ipi_readl() reads off the end of the too-short buf[].

thanks
-- PMM