Re: [PATCH 3/4] hw/m68k/virt: Add a pflash controller for BIOS firmware

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Jiaxun,

On 27/5/24 19:15, Jiaxun Yang wrote:
> Add a 8 MiB pflash controller for BIOS firmware, and boot
> from it if possible.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   hw/m68k/Kconfig                                   |  1 +
>   hw/m68k/virt.c                                    | 44 +++++++++++++++++++++++
>   include/standard-headers/asm-m68k/bootinfo-virt.h |  1 +
>   3 files changed, 46 insertions(+)
> 
> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
> index 4501da56ff6d..f233a5948f19 100644
> --- a/hw/m68k/Kconfig
> +++ b/hw/m68k/Kconfig
> @@ -42,6 +42,7 @@ config M68K_VIRT
>       select M68K_IRQC
>       select FW_CFG_DMA
>       select VIRT_CTRL
> +    select PFLASH_CFI01
>       select GOLDFISH_PIC
>       select GOLDFISH_TTY
>       select GOLDFISH_RTC
> diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
> index 7590e6515ac3..a2eebc0f2243 100644
> --- a/hw/m68k/virt.c
> +++ b/hw/m68k/virt.c
> @@ -8,6 +8,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/datadir.h"
>   #include "qemu/units.h"
>   #include "qemu/guest-random.h"
>   #include "sysemu/sysemu.h"
> @@ -28,6 +29,7 @@
>   #include "sysemu/runstate.h"
>   #include "sysemu/reset.h"
>   
> +#include "hw/block/flash.h"
>   #include "hw/intc/m68k_irqc.h"
>   #include "hw/misc/virt_ctrl.h"
>   #include "hw/char/goldfish_tty.h"
> @@ -97,6 +99,10 @@
>   #define VIRT_XHCI_MMIO_BASE 0xff020000    /* MMIO: 0xff020000 - 0xff023fff */
>   #define VIRT_XHCI_IRQ_BASE  PIC_IRQ(1, 2) /* PIC: #1, IRQ: #2 */
>   
> +#define VIRT_PFLASH_MMIO_BASE 0xff800000      /* MMIO: 0xff800000 - 0xffffffff */
> +#define VIRT_PFLASH_SIZE      0x800000        /* 8 MiB */

Do you need a real RW pflash or a ROM would be enough?

> +#define VIRT_PFLASH_SECTOR_SIZE (128 * KiB)   /* 64 KiB */
> +
>   typedef struct {
>       M68kCPU *cpu;
>       hwaddr initial_pc;
> @@ -139,6 +145,7 @@ static void virt_init(MachineState *machine)
>       const char *initrd_filename = machine->initrd_filename;
>       const char *kernel_cmdline = machine->kernel_cmdline;
>       hwaddr parameters_base;
> +    DriveInfo *dinfo;
>       DeviceState *dev;
>       DeviceState *irqc_dev;
>       DeviceState *pic_dev[VIRT_GF_PIC_NB];
> @@ -165,6 +172,8 @@ static void virt_init(MachineState *machine)
>       cpu = M68K_CPU(cpu_create(machine->cpu_type));
>   
>       reset_info->cpu = cpu;
> +    reset_info->initial_pc = VIRT_PFLASH_MMIO_BASE;
> +    reset_info->initial_stack = ram_size;
>       qemu_register_reset(main_cpu_reset, reset_info);
>   
>       /* RAM */
> @@ -253,6 +262,39 @@ static void virt_init(MachineState *machine)
>                           PIC_GPIO(VIRT_XHCI_IRQ_BASE));
>       }
>   
> +    /* pflash */
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
> +    pflash_cfi01_register(VIRT_PFLASH_MMIO_BASE,
> +                          "virt.pflash0",
> +                           VIRT_PFLASH_SIZE,
> +                           dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> +                           VIRT_PFLASH_SECTOR_SIZE, 4,
> +                           0x89, 0x18, 0, 0, 1);
> +
> +    if (machine->firmware) {
> +        char *fn;
> +        int image_size;
> +
> +        if (drive_get(IF_PFLASH, 0, 0)) {
> +            error_report("The contents of the first flash device may be "
> +                         "specified with -bios or with -drive if=pflash... "
> +                         "but you cannot use both options at once");
> +            exit(1);
> +        }
> +        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, machine->firmware);
> +        if (!fn) {
> +            error_report("Could not find ROM image '%s'", machine->firmware);
> +            exit(1);
> +        }
> +        image_size = load_image_targphys(fn, VIRT_PFLASH_MMIO_BASE,
> +                                         VIRT_PFLASH_SIZE);
> +        g_free(fn);
> +        if (image_size < 0) {
> +            error_report("Could not load ROM image '%s'", machine->firmware);
> +            exit(1);
> +        }
> +    }
> +
>       if (kernel_filename) {
>           CPUState *cs = CPU(cpu);
>           uint64_t high;
> @@ -311,6 +353,8 @@ static void virt_init(MachineState *machine)
>           }
>           BOOTINFO2(param_ptr, BI_VIRT_FW_CFG_BASE,
>                     VIRT_FW_CFG_MMIO_BASE, VIRT_FW_CFG_IRQ_BASE);
> +        BOOTINFO2(param_ptr, BI_VIRT_PFLASH_BASE,
> +                    VIRT_PFLASH_MMIO_BASE, 0);
>   
>           if (kernel_cmdline) {
>               BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
> diff --git a/include/standard-headers/asm-m68k/bootinfo-virt.h b/include/standard-headers/asm-m68k/bootinfo-virt.h
> index 7f90be1aa7bd..21c9a98d2912 100644
> --- a/include/standard-headers/asm-m68k/bootinfo-virt.h
> +++ b/include/standard-headers/asm-m68k/bootinfo-virt.h
> @@ -18,6 +18,7 @@
>   
>   #define BI_VIRT_XHCI_BASE	0x8007
>   #define BI_VIRT_FW_CFG_BASE	0x8008
> +#define BI_VIRT_PFLASH_BASE	0x8009
>   
>   #define VIRT_BOOTI_VERSION	MK_BI_VERSION(2, 0)
>   
>