Re: [PATCH v2 2/2] hw/riscv: sifive_u: Provide a reliable way for bootloader to detect whether it is running in QEMU

[Alistair Francis]

On Thu, Jul 9, 2020 at 3:07 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> The reset vector codes are subject to change, e.g.: with recent
> fw_dynamic type image support, it breaks oreboot again.

This is a recurring problem, I have another patch for Oreboot to fix
the latest breakage.

>
> Add a subregion in the MROM, with the size of machine RAM stored,
> so that we can provide a reliable way for bootloader to detect
> whether it is running in QEMU.

I don't really like this though. I would prefer that we don't
encourage guest software to behave differently on QEMU. I don't think
other upstream boards do this.

Besides Oreboot setting up the clocks are there any other users of this?

Alistair

>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v2:
> - correctly populate the value in little-endian
>
>  hw/riscv/sifive_u.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 3413369..79519d4 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -88,6 +88,7 @@ static const struct MemmapEntry {
>
>  #define OTP_SERIAL          1
>  #define GEM_REVISION        0x10070109
> +#define MROM_RAMSIZE_OFFSET 0xf8
>
>  static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
>      uint64_t mem_size, const char *cmdline)
> @@ -496,6 +497,26 @@ static void sifive_u_machine_init(MachineState *machine)
>      riscv_rom_copy_firmware_info(memmap[SIFIVE_U_MROM].base,
>                                   memmap[SIFIVE_U_MROM].size,
>                                   sizeof(reset_vec), kernel_entry);
> +
> +    /*
> +     * Tell guest the machine ram size at MROM_RAMSIZE_OFFSET.
> +     * On real hardware, the 64-bit value from MROM_RAMSIZE_OFFSET is zero.
> +     * QEMU aware bootloader (e.g.: oreboot, U-Boot) can check value stored
> +     * here to determine whether it is running in QEMU.
> +     */
> +
> +    uint32_t ram_size[2] = {
> +        machine->ram_size,
> +        ((uint64_t)(machine->ram_size)) >> 32
> +    };
> +
> +    /* copy in the ram size in little_endian byte order */
> +    for (i = 0; i < ARRAY_SIZE(ram_size); i++) {
> +        ram_size[i] = cpu_to_le32(ram_size[i]);
> +    }
> +    rom_add_blob_fixed_as("mrom.ram_size", ram_size, sizeof(ram_size),
> +                          memmap[SIFIVE_U_MROM].base + MROM_RAMSIZE_OFFSET,
> +                          &address_space_memory);
>  }
>
>  static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
> --
> 2.7.4
>
>