Re: [PATCH v2 2/2] hw/mips: Add MIPS virt board

[Philippe Mathieu-Daudé]

On 4/3/23 23:38, Jiaxun Yang wrote:
> MIPS virt board is design to utilize existing VirtIO infrastures
> but also comptitable with MIPS's existing internal simulation tools.
>
> It includes virtio-pci, virtio-mmio, pcie gpex, flash rom, fw_cfg,
> goldfish-rtc and MIPS CPS system.
>
> It should be able to cooperate with any MIPS CPU cores.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> v1:
>   - Rename to virt board
>   - Convert BIOS flash to ROM
>   - Cleanups
> v2:
>   - Fix fdt flash
>   - Remove UP variant
> ---
>   MAINTAINERS                             |   7 +
>   configs/devices/mips-softmmu/common.mak |   1 +
>   docs/system/target-mips.rst             |  22 +
>   hw/mips/Kconfig                         |  17 +
>   hw/mips/meson.build                     |   1 +
>   hw/mips/virt.c                          | 916 ++++++++++++++++++++++++
>   6 files changed, 964 insertions(+)
>   create mode 100644 hw/mips/virt.c



> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> index da3a37e215ec..8a753ec2aee2 100644
> --- a/hw/mips/Kconfig
> +++ b/hw/mips/Kconfig
> @@ -59,5 +59,22 @@ config MIPS_BOSTON
>       select AHCI_ICH9
>       select SERIAL
>   
> +config MIPS_VIRT
> +    bool
> +    imply PCI_DEVICES
> +    imply VIRTIO_VGA
> +    imply TEST_DEVICES
> +    select MIPS_CPS
> +    select MIPS_TRICKBOX
> +    select SERIAL
> +    select FW_CFG_MIPS
> +    select GOLDFISH_RTC
> +    select PCI
> +    select PCI_EXPRESS_GENERIC_BRIDGE
> +    select PFLASH_CFI01
> +    select VIRTIO_MMIO
> +    select FW_CFG_DMA
> +    select PLATFORM_BUS
> +
>   config FW_CFG_MIPS
>       bool
> diff --git a/hw/mips/meson.build b/hw/mips/meson.build
> index 900613fc087f..5670c939fa7b 100644
> --- a/hw/mips/meson.build
> +++ b/hw/mips/meson.build
> @@ -1,6 +1,7 @@
>   mips_ss = ss.source_set()
>   mips_ss.add(files('bootloader.c', 'mips_int.c'))
>   mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c'))
> +mips_ss.add(when: 'CONFIG_MIPS_VIRT', if_true: files('virt.c'))
>   mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c', 
> 'loongson3_virt.c'))
>   mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c'))
>   mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
> diff --git a/hw/mips/virt.c b/hw/mips/virt.c
> new file mode 100644
> index 000000000000..e4359f930104
> --- /dev/null
> +++ b/hw/mips/virt.c
> @@ -0,0 +1,916 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later

Besides the // comment which is not QEMU style, can we
use GPL-2.0-or-later for files used by final binaries?
IIUC LGPL is for lib-erizable files such (lib)tcg.

> +#include "qom/object.h"
> +#include <libfdt.h>
> +
> +#define TYPE_MIPS_VIRT_MACHINE MACHINE_TYPE_NAME("virt")

So now every variant requires libfdt available on the host,
otherwise building with --target-list=mips-softmmu,mipsel-softmmu
on a host without libfdt:

../../hw/mips/virt.c:47:10: fatal error: 'libfdt.h' file not found
#include <libfdt.h>
         ^~~~~~~~~~

So we can use:
-- >8 --
diff --git a/configs/targets/mips-softmmu.mak 
b/configs/targets/mips-softmmu.mak
index 7787a4d94c..a5c1db82c9 100644
--- a/configs/targets/mips-softmmu.mak
+++ b/configs/targets/mips-softmmu.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=mips
 TARGET_ALIGNED_ONLY=y
 TARGET_BIG_ENDIAN=y
 TARGET_SUPPORTS_MTTCG=y
+TARGET_NEED_FDT=y
diff --git a/configs/targets/mips64-softmmu.mak 
b/configs/targets/mips64-softmmu.mak
index 568d66650c..398e0fc244 100644
--- a/configs/targets/mips64-softmmu.mak
+++ b/configs/targets/mips64-softmmu.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=mips64
 TARGET_BASE_ARCH=mips
 TARGET_ALIGNED_ONLY=y
 TARGET_BIG_ENDIAN=y
+TARGET_NEED_FDT=y
diff --git a/configs/targets/mipsel-softmmu.mak 
b/configs/targets/mipsel-softmmu.mak
index c7c41f4fb7..3ddebca575 100644
--- a/configs/targets/mipsel-softmmu.mak
+++ b/configs/targets/mipsel-softmmu.mak
@@ -1,3 +1,4 @@
 TARGET_ARCH=mips
 TARGET_ALIGNED_ONLY=y
 TARGET_SUPPORTS_MTTCG=y
+TARGET_NEED_FDT=y
---

But if we can't install libfdt (non-root?) then we can't build any MIPS
target.

$ git grep -l TARGET_NEED_FDT configs/targets/*softmmu.mak | wc -l
      18
$ git grep -L TARGET_NEED_FDT configs/targets/*softmmu.mak | wc -l
      13

I guess that's OK...