diff mbox series

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

Message ID 1594289144-24723-2-git-send-email-bmeng.cn@gmail.com
State New
Headers show
Series [v2,1/2] hw/riscv: Modify MROM size to end at 0x10000 | expand

Commit Message

Bin Meng July 9, 2020, 10:05 a.m. UTC
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.

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.

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(+)

Comments

Alistair Francis July 9, 2020, 10:09 p.m. UTC | #1
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
>
>
Palmer Dabbelt July 10, 2020, 12:45 a.m. UTC | #2
On Thu, 09 Jul 2020 15:09:18 PDT (-0700), alistair23@gmail.com wrote:
> 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.

I agree.  If you want an explicitly virtual board, use the virt board.  Users
of sifive_u are presumably trying to do their best to test against what the
hardware does without actually using the hardware.  Otherwise there should be
no reason to use the sifive_u board, as it's just sticking a layer of
complexity in the middle of everything.

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

IIRC we have a scheme for handling the clock setup in QEMU where we accept
pretty much any control write and then just return reads that say the PLLs have
locked.  I'd be in favor of improving the scheme to improve compatibility with
the actual hardware, but adding some way for programs to skip the clocks
because they know they're in QEMU seems like the wrong way to go.

> 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
>>
>>
Bin Meng July 10, 2020, 12:48 a.m. UTC | #3
Hi Alistair,

On Fri, Jul 10, 2020 at 6:19 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> 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.
>

Can Oreboot be updated to remove the QEMU detection?

> >
> > 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?

I don't really have any specific reason, except for testing U-Boot SPL
by relaxing the requirement of hardcoding the memory to 8G "-m 8G" as
I indicated in the commit message below:

commit 3eaea6eb4e534f7b87c6eca808149bb671976800
Author: Bin Meng <bin.meng@windriver.com>
Date:   Mon Jun 15 17:50:41 2020 -0700

    hw/riscv: sifive_u: Add a dummy DDR memory controller device

    It is enough to simply map the SiFive FU540 DDR memory controller
    into the MMIO space using create_unimplemented_device(), to make
    the upstream U-Boot v2020.07 DDR memory initialization codes happy.

    Note we do not generate device tree fragment for the DDR memory
    controller. Since the controller data in device tree consumes a
    very large space (see fu540-hifive-unleashed-a00-ddr.dtsi in the
    U-Boot source), and it is only needed by U-Boot SPL but not any
    operating system, we choose not to generate the fragment here.
    This also means when testing with U-Boot SPL, the device tree has
    to come from U-Boot SPL itself, but not the one generated by QEMU
    on the fly. The memory has to be set to 8GiB to match the real
    HiFive Unleashed board when invoking QEMU (-m 8G).

Cc'ing Pragnesh and Sagar as they wanted to test U-Boot SPL with QEMU
and talked to me the other day.

Regards,
Bin
Bin Meng July 10, 2020, 12:50 a.m. UTC | #4
Hi Palmer,

On Fri, Jul 10, 2020 at 8:45 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
>
> On Thu, 09 Jul 2020 15:09:18 PDT (-0700), alistair23@gmail.com wrote:
> > 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.
>
> I agree.  If you want an explicitly virtual board, use the virt board.  Users
> of sifive_u are presumably trying to do their best to test against what the
> hardware does without actually using the hardware.  Otherwise there should be
> no reason to use the sifive_u board, as it's just sticking a layer of
> complexity in the middle of everything.

Understood. Then let's drop this patch.

>
> > Besides Oreboot setting up the clocks are there any other users of this?
>
> IIRC we have a scheme for handling the clock setup in QEMU where we accept
> pretty much any control write and then just return reads that say the PLLs have
> locked.  I'd be in favor of improving the scheme to improve compatibility with
> the actual hardware, but adding some way for programs to skip the clocks
> because they know they're in QEMU seems like the wrong way to go.
>

Yep, that's my question to Oreboot too.

U-Boot SPL can boot with QEMU and no problem was seen with clock
settings in PRCI model in QEMU.

Regards,
Bin
Alistair Francis July 11, 2020, 3:53 p.m. UTC | #5
On Thu, Jul 9, 2020 at 5:48 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Fri, Jul 10, 2020 at 6:19 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > 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.
> >
>
> Can Oreboot be updated to remove the QEMU detection?

In general I think it should be.

Right now it's not critical to do. I think from a QEMU perspective we
have finished changing the "ROM" code so after this release we can
update Oreboot and then it should settle down again.

>
> > >
> > > 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?
>
> I don't really have any specific reason, except for testing U-Boot SPL
> by relaxing the requirement of hardcoding the memory to 8G "-m 8G" as
> I indicated in the commit message below:

Yeah, I think that's just something we will have to deal with. If the
guest expects 8GB and doesn't check the device tree passed to it then
the user has to create 8GB of memory.

Alistair

>
> commit 3eaea6eb4e534f7b87c6eca808149bb671976800
> Author: Bin Meng <bin.meng@windriver.com>
> Date:   Mon Jun 15 17:50:41 2020 -0700
>
>     hw/riscv: sifive_u: Add a dummy DDR memory controller device
>
>     It is enough to simply map the SiFive FU540 DDR memory controller
>     into the MMIO space using create_unimplemented_device(), to make
>     the upstream U-Boot v2020.07 DDR memory initialization codes happy.
>
>     Note we do not generate device tree fragment for the DDR memory
>     controller. Since the controller data in device tree consumes a
>     very large space (see fu540-hifive-unleashed-a00-ddr.dtsi in the
>     U-Boot source), and it is only needed by U-Boot SPL but not any
>     operating system, we choose not to generate the fragment here.
>     This also means when testing with U-Boot SPL, the device tree has
>     to come from U-Boot SPL itself, but not the one generated by QEMU
>     on the fly. The memory has to be set to 8GiB to match the real
>     HiFive Unleashed board when invoking QEMU (-m 8G).
>
> Cc'ing Pragnesh and Sagar as they wanted to test U-Boot SPL with QEMU
> and talked to me the other day.
>
> Regards,
> Bin
Alistair Francis July 11, 2020, 3:54 p.m. UTC | #6
On Thu, Jul 9, 2020 at 5:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Palmer,
>
> On Fri, Jul 10, 2020 at 8:45 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
> >
> > On Thu, 09 Jul 2020 15:09:18 PDT (-0700), alistair23@gmail.com wrote:
> > > 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.
> >
> > I agree.  If you want an explicitly virtual board, use the virt board.  Users
> > of sifive_u are presumably trying to do their best to test against what the
> > hardware does without actually using the hardware.  Otherwise there should be
> > no reason to use the sifive_u board, as it's just sticking a layer of
> > complexity in the middle of everything.
>
> Understood. Then let's drop this patch.
>
> >
> > > Besides Oreboot setting up the clocks are there any other users of this?
> >
> > IIRC we have a scheme for handling the clock setup in QEMU where we accept
> > pretty much any control write and then just return reads that say the PLLs have
> > locked.  I'd be in favor of improving the scheme to improve compatibility with
> > the actual hardware, but adding some way for programs to skip the clocks
> > because they know they're in QEMU seems like the wrong way to go.
> >
>
> Yep, that's my question to Oreboot too.
>
> U-Boot SPL can boot with QEMU and no problem was seen with clock
> settings in PRCI model in QEMU.

I don't think it's an unsolvable problem. There is just little work on
Oreboot to run on QEMU. I can dig into it a bit and see if I can find
a better fix on the Oreboot side.

Alistair

>
> Regards,
> Bin
Bin Meng July 13, 2020, 1:14 a.m. UTC | #7
Hi Alistair,

On Sun, Jul 12, 2020 at 12:03 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, Jul 9, 2020 at 5:48 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Alistair,
> >
> > On Fri, Jul 10, 2020 at 6:19 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > 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.
> > >
> >
> > Can Oreboot be updated to remove the QEMU detection?
>
> In general I think it should be.
>
> Right now it's not critical to do. I think from a QEMU perspective we
> have finished changing the "ROM" code so after this release we can
> update Oreboot and then it should settle down again.
>
> >
> > > >
> > > > 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?
> >
> > I don't really have any specific reason, except for testing U-Boot SPL
> > by relaxing the requirement of hardcoding the memory to 8G "-m 8G" as
> > I indicated in the commit message below:
>
> Yeah, I think that's just something we will have to deal with. If the
> guest expects 8GB and doesn't check the device tree passed to it then
> the user has to create 8GB of memory.
>

Note there are two reasons that why "-m 8G" is used to test U-Boot SPL:

1. U-Boot DDR driver is hardcoding memory to 8G, which I had a fix
locally and will send U-Boot upstream soon.
2. U-Boot SPL has to use its own device tree because we don't want to
update QEMU to include all the DDR register settings in the device
tree which is very big.

Why I wanted to add this patch here is that I wanted to dynamically
patch U-Boot SPL DT to use the actual ram size that QEMU instantiates.
This way we can avoid editing U-Boot SPL DT to set the actual memory
size.

Regards,
Bin
Bin Meng July 13, 2020, 1:16 a.m. UTC | #8
Hi Alistair,

On Sun, Jul 12, 2020 at 12:04 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, Jul 9, 2020 at 5:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Palmer,
> >
> > On Fri, Jul 10, 2020 at 8:45 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
> > >
> > > On Thu, 09 Jul 2020 15:09:18 PDT (-0700), alistair23@gmail.com wrote:
> > > > 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.
> > >
> > > I agree.  If you want an explicitly virtual board, use the virt board.  Users
> > > of sifive_u are presumably trying to do their best to test against what the
> > > hardware does without actually using the hardware.  Otherwise there should be
> > > no reason to use the sifive_u board, as it's just sticking a layer of
> > > complexity in the middle of everything.
> >
> > Understood. Then let's drop this patch.
> >
> > >
> > > > Besides Oreboot setting up the clocks are there any other users of this?
> > >
> > > IIRC we have a scheme for handling the clock setup in QEMU where we accept
> > > pretty much any control write and then just return reads that say the PLLs have
> > > locked.  I'd be in favor of improving the scheme to improve compatibility with
> > > the actual hardware, but adding some way for programs to skip the clocks
> > > because they know they're in QEMU seems like the wrong way to go.
> > >
> >
> > Yep, that's my question to Oreboot too.
> >
> > U-Boot SPL can boot with QEMU and no problem was seen with clock
> > settings in PRCI model in QEMU.
>
> I don't think it's an unsolvable problem. There is just little work on
> Oreboot to run on QEMU. I can dig into it a bit and see if I can find
> a better fix on the Oreboot side.
>

Can we remove the QEMU detect logic completely in Oreboot? Except the
QSPI controller QEMU should be able to run Oreboot since it runs
U-Boot SPL.

Regards,
Bin
Alistair Francis July 14, 2020, 12:42 a.m. UTC | #9
On Sun, Jul 12, 2020 at 6:16 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Sun, Jul 12, 2020 at 12:04 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Thu, Jul 9, 2020 at 5:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Palmer,
> > >
> > > On Fri, Jul 10, 2020 at 8:45 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
> > > >
> > > > On Thu, 09 Jul 2020 15:09:18 PDT (-0700), alistair23@gmail.com wrote:
> > > > > 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.
> > > >
> > > > I agree.  If you want an explicitly virtual board, use the virt board.  Users
> > > > of sifive_u are presumably trying to do their best to test against what the
> > > > hardware does without actually using the hardware.  Otherwise there should be
> > > > no reason to use the sifive_u board, as it's just sticking a layer of
> > > > complexity in the middle of everything.
> > >
> > > Understood. Then let's drop this patch.
> > >
> > > >
> > > > > Besides Oreboot setting up the clocks are there any other users of this?
> > > >
> > > > IIRC we have a scheme for handling the clock setup in QEMU where we accept
> > > > pretty much any control write and then just return reads that say the PLLs have
> > > > locked.  I'd be in favor of improving the scheme to improve compatibility with
> > > > the actual hardware, but adding some way for programs to skip the clocks
> > > > because they know they're in QEMU seems like the wrong way to go.
> > > >
> > >
> > > Yep, that's my question to Oreboot too.
> > >
> > > U-Boot SPL can boot with QEMU and no problem was seen with clock
> > > settings in PRCI model in QEMU.
> >
> > I don't think it's an unsolvable problem. There is just little work on
> > Oreboot to run on QEMU. I can dig into it a bit and see if I can find
> > a better fix on the Oreboot side.
> >
>
> Can we remove the QEMU detect logic completely in Oreboot? Except the
> QSPI controller QEMU should be able to run Oreboot since it runs
> U-Boot SPL.

That is the eventual goal.

Alistair

>
> Regards,
> Bin
diff mbox series

Patch

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)