Re: [PATCH v2 2/2] hw/mips/loongson3_virt: Add TCG SMP support

[Philippe Mathieu-Daudé]

Hi Jiaxun,

On 1/14/21 2:31 AM, Jiaxun Yang wrote:
> loongson3_virt has KVM SMP support in kenrel.
> This patch adds TCG SMP support by enable IPI controller
> for machine.
> 
> Note that TCG SMP can only support up to 4 CPUs as we
> didn't implement multi-node support.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/mips/loongson3_bootp.h |  1 +
>  hw/mips/loongson3_virt.c  | 20 +++++++++++++++++++-
>  hw/mips/Kconfig           |  1 +
>  3 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mips/loongson3_bootp.h b/hw/mips/loongson3_bootp.h
> index 09f8480abf..4756aa44f6 100644
> --- a/hw/mips/loongson3_bootp.h
> +++ b/hw/mips/loongson3_bootp.h
> @@ -210,6 +210,7 @@ enum {
>      VIRT_PCIE_ECAM,
>      VIRT_BIOS_ROM,
>      VIRT_UART,
> +    VIRT_IPIS,
>      VIRT_LIOINTC,
>      VIRT_PCIE_MMIO,
>      VIRT_HIGHMEM
> diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
> index d4a82fa536..0684a035b0 100644
> --- a/hw/mips/loongson3_virt.c
> +++ b/hw/mips/loongson3_virt.c
> @@ -35,6 +35,7 @@
>  #include "hw/boards.h"
>  #include "hw/char/serial.h"
>  #include "hw/intc/loongson_liointc.h"
> +#include "hw/intc/loongson_ipi.h"
>  #include "hw/mips/mips.h"
>  #include "hw/mips/cpudevs.h"
>  #include "hw/mips/fw_cfg.h"
> @@ -59,6 +60,7 @@
>  
>  #define PM_CNTL_MODE          0x10
>  
> +#define LOONGSON_TCG_MAX_VCPUS  4
>  #define LOONGSON_MAX_VCPUS      16

A GS464V node has 4 cores, right? What about:

#define LOONGSON_VCPUS_PER_NODE 4

#define LOONGSON_NODES_MAX_TCG  1
#define LOONGSON_NODES_MAX_KVM  4

>  
>  /*
> @@ -71,6 +73,7 @@
>  #define UART_IRQ            0
>  #define RTC_IRQ             1
>  #define PCIE_IRQ_BASE       2

Can you add a definition for the number 6? Maybe:

   #define IPI_IRQ             6

> +#define IPI_REG_SPACE       0x100
>  
>  const struct MemmapEntry virt_memmap[] = {
>      [VIRT_LOWMEM] =      { 0x00000000,    0x10000000 },
> @@ -81,6 +84,7 @@ const struct MemmapEntry virt_memmap[] = {
>      [VIRT_PCIE_ECAM] =   { 0x1a000000,     0x2000000 },
>      [VIRT_BIOS_ROM] =    { 0x1fc00000,      0x200000 },
>      [VIRT_UART] =        { 0x1fe001e0,           0x8 },
> +    [VIRT_IPIS] =        { 0x3ff01000,         0x400 },
>      [VIRT_LIOINTC] =     { 0x3ff01400,          0x64 },
>      [VIRT_PCIE_MMIO] =   { 0x40000000,    0x40000000 },
>      [VIRT_HIGHMEM] =     { 0x80000000,           0x0 }, /* Variable */
> @@ -495,6 +499,10 @@ static void mips_loongson3_virt_init(MachineState 
> *machine)
>              error_report("Loongson-3/TCG needs cpu type Loongson-3A1000");
>              exit(1);
>          }
> +        if (machine->smp.cpus > LOONGSON_TCG_MAX_VCPUS) {
> +            error_report("Loongson-3/TCG supports up to 4 CPUs");
> +            exit(1);
> +        }
>      } else {
>          if (!machine->cpu_type) {
>              machine->cpu_type = MIPS_CPU_TYPE_NAME("Loongson-3A4000");
> @@ -545,7 +553,17 @@ static void mips_loongson3_virt_init(MachineState 
> *machine)
>          qemu_register_reset(main_cpu_reset, cpu);
>  
>          if (i >= 4) {
> -            continue; /* Only node-0 can be connected to LIOINTC */
> +            continue; /* Only node-0 can be connected to LIOINTC and IPI */
> +        }
> +
> +        if (!kvm_enabled()) {
> +            /* IPI is handled by kernel for KVM */
> +            DeviceState *ipi;
> +            ipi = qdev_new(TYPE_LOONGSON_IPI);
> +            sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
> +            sysbus_mmio_map(SYS_BUS_DEVICE(ipi), 0,
> +                            virt_memmap[VIRT_IPIS].base + IPI_REG_SPACE * i);
> +            sysbus_connect_irq(SYS_BUS_DEVICE(ipi), 0, cpu->env.irq[6]);

While this works, it is very fragile. If multiple IRQs share the same
CPU pin, the better way is to use an OR gate (modeled as TYPE_OR_IRQ
device).

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

Oops, we already use it without OR gate :/

Regards,

Phil.