Question about RISC-V brom register a1 set value

Question about RISC-V brom register a1 set value

[Eric Chan]

[-- Attachment #1: Type: text/plain, Size: 3545 bytes --]

Hi, qemu

As I know, brom will pass 3 parameters to the next stage bootloader, ex:
openSBI.
a0 will pass hartid, a2 will pass fw_dynamic_info start address.
although a1 doesn't use directly in openSBI.
a1 read value is determined in compile time rather than read from the
original a1 that passes from brom.
In qemu/hw/riscv/boot.c
both 32bit and 64bit machines read 4byte that offset 32byte from the brom
start address.

for 64 bits machine: a1 read low 32bit data member magic of fw_dynamic_info,
the value will same as FW_DYNAMIC_INFO_MAGIC_VALUE because risc-v is little
endian.

for 32bits machine: each data member of fw_dynamic_info is 4 bytes, so a1
will read the version rather than magic.

Do the 32bit and 64bit pass different parameters are expected?
If it is not expected, I guess the original version is 64bit machine, and
then supports 32bit but misses this detail, I hope I can have an
opportunity to fix this problem.
If it is expected, why they must be done?

Thanks,
Eric Chan

qemu/include/hw/riscv/boot_opensbi.h
#define FW_DYNAMIC_INFO_MAGIC_VALUE     0x4942534f
qemu/hw/riscv/boot.c
void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *
harts,
                               hwaddr start_addr,
                               hwaddr rom_base, hwaddr rom_size,
                               uint64_t kernel_entry,
                               uint64_t fdt_load_addr)
{
    int i;
    uint32_t start_addr_hi32 = 0x00000000;
    uint32_t fdt_load_addr_hi32 = 0x00000000;

    if (!riscv_is_32bit(harts)) {
        start_addr_hi32 = start_addr >> 32;
        fdt_load_addr_hi32 = fdt_load_addr >> 32;
    }
    /* reset vector */
    uint32_t reset_vec[10] = {
        0x00000297,                  /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
        0x02828613,                  /*     addi   a2, t0, %pcrel_lo(1b) */
        0xf1402573,                  /*     csrr   a0, mhartid  */
        0,
        0,
        0x00028067,                  /*     jr     t0 */
        start_addr,                  /* start: .dword */
        start_addr_hi32,
        fdt_load_addr,               /* fdt_laddr: .dword */
        fdt_load_addr_hi32,
                                     /* fw_dyn: */
    };
    if (riscv_is_32bit(harts)) {
        reset_vec[3] = 0x0202a583;   /*     lw     a1, 32(t0) */
        reset_vec[4] = 0x0182a283;   /*     lw     t0, 24(t0) */
    } else {
        reset_vec[3] = 0x0202b583;   /*     ld     a1, 32(t0) */
        reset_vec[4] = 0x0182b283;   /*     ld     t0, 24(t0) */
    }

    /* copy in the reset vector in little_endian byte order */
    for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
        reset_vec[i] = cpu_to_le32(reset_vec[i]);
    }
    rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
                          rom_base, &address_space_memory);
    riscv_rom_copy_firmware_info(machine, rom_base, rom_size, sizeof
(reset_vec),
                                 kernel_entry);
}

opensbi/firmware/fw_dynamic.S
fw_boot_hart:
        /* Sanity checks */
        li      a1, FW_DYNAMIC_INFO_MAGIC_VALUE
        REG_L   a0, FW_DYNAMIC_INFO_MAGIC_OFFSET(a2)
        bne     a0, a1, _bad_dynamic_info
        li      a1, FW_DYNAMIC_INFO_VERSION_MAX
        REG_L   a0, FW_DYNAMIC_INFO_VERSION_OFFSET(a2)
        bgt     a0, a1, _bad_dynamic_info

        /* Read boot HART id */
        li      a1, FW_DYNAMIC_INFO_VERSION_2
        blt     a0, a1, 2f
        REG_L   a0, FW_DYNAMIC_INFO_BOOT_HART_OFFSET(a2)
        ret
2:      li      a0, -1
        ret

[-- Attachment #2: Type: text/html, Size: 9408 bytes --]

Re: Question about RISC-V brom register a1 set value

[Philippe Mathieu-Daudé via]

Cc'ing the RISC-V specific mailing list.

On 1/10/22 01:48, Eric Chan wrote:
> Hi, qemu
> 
> As I know, brom will pass 3 parameters to the next stage bootloader, ex: 
> openSBI.
> a0 will pass hartid, a2 will pass fw_dynamic_info start address.
> although a1 doesn't use directly in openSBI.
> a1 read value is determined in compile time rather than read from the 
> original a1 that passes from brom.
> In qemu/hw/riscv/boot.c
> both 32bit and 64bit machines read 4byte that offset 32byte from the 
> brom start address.
> 
> for 64 bits machine: a1 read low 32bit data member magic of fw_dynamic_info,
> the value will same as FW_DYNAMIC_INFO_MAGIC_VALUE because risc-v is 
> little endian.
> 
> for 32bits machine: each data member of fw_dynamic_info is 4 bytes, so 
> a1 will read the version rather than magic.
> 
> Do the 32bit and 64bit pass different parameters are expected?
> If it is not expected, I guess the original version is 64bit machine, 
> and then supports 32bit but misses this detail, I hope I can have an 
> opportunity to fix this problem.
> If it is expected, why they must be done?
> 
> Thanks,
> Eric Chan
> 
> qemu/include/hw/riscv/boot_opensbi.h
> #define FW_DYNAMIC_INFO_MAGIC_VALUE     0x4942534f
> qemu/hw/riscv/boot.c
> void riscv_setup_rom_reset_vec(MachineState *machine, 
> RISCVHartArrayState *harts,
>                                 hwaddr start_addr,
>                                 hwaddr rom_base, hwaddr rom_size,
> uint64_t kernel_entry,
> uint64_t fdt_load_addr)
> {
> int i;
> uint32_t start_addr_hi32 = 0x00000000;
> uint32_t fdt_load_addr_hi32 = 0x00000000;
> 
> if (!riscv_is_32bit(harts)) {
>          start_addr_hi32 = start_addr >> 32;
>          fdt_load_addr_hi32 = fdt_load_addr >> 32;
>      }
>      /* reset vector */
> uint32_t reset_vec[10] = {
> 0x00000297,                 /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
> 0x02828613,                 /*     addi   a2, t0, %pcrel_lo(1b) */
> 0xf1402573,                 /*     csrr   a0, mhartid  */
> 0,
> 0,
> 0x00028067,                 /*     jr     t0 */
>          start_addr,                 /* start: .dword */
>          start_addr_hi32,
>          fdt_load_addr,              /* fdt_laddr: .dword */
>          fdt_load_addr_hi32,
>                                       /* fw_dyn: */
>      };
> if (riscv_is_32bit(harts)) {
> reset_vec[3] = 0x0202a583;  /*     lw     a1, 32(t0) */
> reset_vec[4] = 0x0182a283;  /*     lw     t0, 24(t0) */
>      } else {
> reset_vec[3] = 0x0202b583;  /*     ld     a1, 32(t0) */
> reset_vec[4] = 0x0182b283;  /*     ld     t0, 24(t0) */
>      }
> 
>      /* copy in the reset vector in little_endian byte order */
> for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
> reset_vec[i] = cpu_to_le32(reset_vec[i]);
>      }
> rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
>                            rom_base, &address_space_memory);
> riscv_rom_copy_firmware_info(machine, rom_base, rom_size, sizeof(reset_vec),
>                                   kernel_entry);
> }
> 
> opensbi/firmware/fw_dynamic.S
> fw_boot_hart:
> /* Sanity checks */
> li a1, FW_DYNAMIC_INFO_MAGIC_VALUE
>          REG_L a0, FW_DYNAMIC_INFO_MAGIC_OFFSET(a2)
> bne a0, a1, _bad_dynamic_info
> li a1, FW_DYNAMIC_INFO_VERSION_MAX
>          REG_L a0, FW_DYNAMIC_INFO_VERSION_OFFSET(a2)
> bgt a0, a1, _bad_dynamic_info
> 
> /* Read boot HART id */
> li a1, FW_DYNAMIC_INFO_VERSION_2
> blt a0, a1, 2f
>          REG_L a0, FW_DYNAMIC_INFO_BOOT_HART_OFFSET(a2)
> ret
> 2: li a0, -1
> ret

回复：Question about RISC-V brom register a1 set value

[刘志伟]

[-- Attachment #1: Type: text/plain, Size: 3633 bytes --]

I think the a1 from QEMU reset vector is the device tree(fdt_laddr) though I don't know if it is directly used by the OpenSBI or just passed to the Linux kernel.
Thanks,
Zhiwei
------------------------------------------------------------------
发件人：Philippe Mathieu-Daudé via <qemu-devel@nongnu.org>
发送时间：2022年10月3日(星期一) 20:37
收件人：Eric Chan <e14002270@gmail.com>; qemu-devel <qemu-devel@nongnu.org>
抄　送：qemu-riscv <qemu-riscv@nongnu.org>
主　题：Re: Question about RISC-V brom register a1 set value
Cc'ing the RISC-V specific mailing list.
On 1/10/22 01:48, Eric Chan wrote:
> Hi, qemu
> 
> As I know, brom will pass 3 parameters to the next stage bootloader, ex: 
> openSBI.
> a0 will pass hartid, a2 will pass fw_dynamic_info start address.
> although a1 doesn't use directly in openSBI.
> a1 read value is determined in compile time rather than read from the 
> original a1 that passes from brom.
> In qemu/hw/riscv/boot.c
> both 32bit and 64bit machines read 4byte that offset 32byte from the 
> brom start address.
> 
> for 64 bits machine: a1 read low 32bit data member magic of fw_dynamic_info,
> the value will same as FW_DYNAMIC_INFO_MAGIC_VALUE because risc-v is 
> little endian.
> 
> for 32bits machine: each data member of fw_dynamic_info is 4 bytes, so 
> a1 will read the version rather than magic.
> 
> Do the 32bit and 64bit pass different parameters are expected?
> If it is not expected, I guess the original version is 64bit machine, 
> and then supports 32bit but misses this detail, I hope I can have an 
> opportunity to fix this problem.
> If it is expected, why they must be done?
> 
> Thanks,
> Eric Chan
> 
> qemu/include/hw/riscv/boot_opensbi.h
> #define FW_DYNAMIC_INFO_MAGIC_VALUE 0x4942534f
> qemu/hw/riscv/boot.c
> void riscv_setup_rom_reset_vec(MachineState *machine, 
> RISCVHartArrayState *harts,
> hwaddr start_addr,
> hwaddr rom_base, hwaddr rom_size,
> uint64_t kernel_entry,
> uint64_t fdt_load_addr)
> {
> int i;
> uint32_t start_addr_hi32 = 0x00000000;
> uint32_t fdt_load_addr_hi32 = 0x00000000;
> 
> if (!riscv_is_32bit(harts)) {
> start_addr_hi32 = start_addr >> 32;
> fdt_load_addr_hi32 = fdt_load_addr >> 32;
> }
> /* reset vector */
> uint32_t reset_vec[10] = {
> 0x00000297, /* 1: auipc t0, %pcrel_hi(fw_dyn) */
> 0x02828613, /* addi a2, t0, %pcrel_lo(1b) */
> 0xf1402573, /* csrr a0, mhartid */
> 0,
> 0,
> 0x00028067, /* jr t0 */
> start_addr, /* start: .dword */
> start_addr_hi32,
> fdt_load_addr, /* fdt_laddr: .dword */
> fdt_load_addr_hi32,
> /* fw_dyn: */
> };
> if (riscv_is_32bit(harts)) {
> reset_vec[3] = 0x0202a583; /* lw a1, 32(t0) */
> reset_vec[4] = 0x0182a283; /* lw t0, 24(t0) */
> } else {
> reset_vec[3] = 0x0202b583; /* ld a1, 32(t0) */
> reset_vec[4] = 0x0182b283; /* ld t0, 24(t0) */
> }
> 
> /* copy in the reset vector in little_endian byte order */
> for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
> reset_vec[i] = cpu_to_le32(reset_vec[i]);
> }
> rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> rom_base, &address_space_memory);
> riscv_rom_copy_firmware_info(machine, rom_base, rom_size, sizeof(reset_vec),
> kernel_entry);
> }
> 
> opensbi/firmware/fw_dynamic.S
> fw_boot_hart:
> /* Sanity checks */
> li a1, FW_DYNAMIC_INFO_MAGIC_VALUE
> REG_L a0, FW_DYNAMIC_INFO_MAGIC_OFFSET(a2)
> bne a0, a1, _bad_dynamic_info
> li a1, FW_DYNAMIC_INFO_VERSION_MAX
> REG_L a0, FW_DYNAMIC_INFO_VERSION_OFFSET(a2)
> bgt a0, a1, _bad_dynamic_info
> 
> /* Read boot HART id */
> li a1, FW_DYNAMIC_INFO_VERSION_2
> blt a0, a1, 2f
> REG_L a0, FW_DYNAMIC_INFO_BOOT_HART_OFFSET(a2)
> ret
> 2: li a0, -1
> ret

[-- Attachment #2: Type: text/html, Size: 6718 bytes --]

Re: Question about RISC-V brom register a1 set value

[Eric Chan]

[-- Attachment #1: Type: text/plain, Size: 4663 bytes --]

Hi  Zhiwei,

thanks for your reply, I make a mistake, t0 is 0x1000 so both
lw     a1, 32(t0) or ld     a1, 32(t0) just read from the brom offset
32byte.

Thanks,
Eric Chan

刘志伟 <zhiwei_liu@c-sky.com> 於 2022年10月5日 週三 下午5:47寫道：

> I think the a1 from QEMU reset vector is the device tree(fdt_laddr) though
> I don't know if it is directly used by the OpenSBI or just passed to the
> Linux kernel.
>
> Thanks,
> Zhiwei
>
> ------------------------------------------------------------------
> 发件人：Philippe Mathieu-Daudé via <qemu-devel@nongnu.org>
> 发送时间：2022年10月3日(星期一) 20:37
> 收件人：Eric Chan <e14002270@gmail.com>; qemu-devel <qemu-devel@nongnu.org>
> 抄 送：qemu-riscv <qemu-riscv@nongnu.org>
> 主 题：Re: Question about RISC-V brom register a1 set value
>
> Cc'ing the RISC-V specific mailing list.
>
> On 1/10/22 01:48, Eric Chan wrote:
> > Hi, qemu
> >
> > As I know, brom will pass 3 parameters to the next stage bootloader, ex:
> > openSBI.
> > a0 will pass hartid, a2 will pass fw_dynamic_info start address.
> > although a1 doesn't use directly in openSBI.
> > a1 read value is determined in compile time rather than read from the
> > original a1 that passes from brom.
> > In qemu/hw/riscv/boot.c
> > both 32bit and 64bit machines read 4byte that offset 32byte from the
> > brom start address.
> >
> > for 64 bits machine: a1 read low 32bit data member magic of
> fw_dynamic_info,
> > the value will same as FW_DYNAMIC_INFO_MAGIC_VALUE because risc-v is
> > little endian.
> >
> > for 32bits machine: each data member of fw_dynamic_info is 4 bytes, so
> > a1 will read the version rather than magic.
> >
> > Do the 32bit and 64bit pass different parameters are expected?
> > If it is not expected, I guess the original version is 64bit machine,
> > and then supports 32bit but misses this detail, I hope I can have an
> > opportunity to fix this problem.
> > If it is expected, why they must be done?
> >
> > Thanks,
> > Eric Chan
> >
> > qemu/include/hw/riscv/boot_opensbi.h
> > #define FW_DYNAMIC_INFO_MAGIC_VALUE     0x4942534f
> > qemu/hw/riscv/boot.c
> > void riscv_setup_rom_reset_vec(MachineState *machine,
> > RISCVHartArrayState *harts,
> >                                 hwaddr start_addr,
> >                                 hwaddr rom_base, hwaddr rom_size,
> > uint64_t kernel_entry,
> > uint64_t fdt_load_addr)
> > {
> > int i;
> > uint32_t start_addr_hi32 = 0x00000000;
> > uint32_t fdt_load_addr_hi32 = 0x00000000;
> >
> > if (!riscv_is_32bit(harts)) {
> >          start_addr_hi32 = start_addr >> 32;
> >          fdt_load_addr_hi32 = fdt_load_addr >> 32;
> >      }
> >      /* reset vector */
> > uint32_t reset_vec[10] = {
> > 0x00000297,                 /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
> > 0x02828613,                 /*     addi   a2, t0, %pcrel_lo(1b) */
> > 0xf1402573,                 /*     csrr   a0, mhartid  */
> > 0,
> > 0,
> > 0x00028067,                 /*     jr     t0 */
> >          start_addr,                 /* start: .dword */
> >          start_addr_hi32,
> >          fdt_load_addr,              /* fdt_laddr: .dword */
> >          fdt_load_addr_hi32,
> >                                       /* fw_dyn: */
> >      };
> > if (riscv_is_32bit(harts)) {
> > reset_vec[3] = 0x0202a583;  /*     lw     a1, 32(t0) */
> > reset_vec[4] = 0x0182a283;  /*     lw     t0, 24(t0) */
> >      } else {
> > reset_vec[3] = 0x0202b583;  /*     ld     a1, 32(t0) */
> > reset_vec[4] = 0x0182b283;  /*     ld     t0, 24(t0) */
> >      }
> >
> >      /* copy in the reset vector in little_endian byte order */
> > for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
> > reset_vec[i] = cpu_to_le32(reset_vec[i]);
> >      }
> > rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> >                            rom_base, &address_space_memory);
> > riscv_rom_copy_firmware_info(machine, rom_base, rom_size,
> sizeof(reset_vec),
> >                                   kernel_entry);
> > }
> >
> > opensbi/firmware/fw_dynamic.S
> > fw_boot_hart:
> > /* Sanity checks */
> > li a1, FW_DYNAMIC_INFO_MAGIC_VALUE
> >          REG_L a0, FW_DYNAMIC_INFO_MAGIC_OFFSET(a2)
> > bne a0, a1, _bad_dynamic_info
> > li a1, FW_DYNAMIC_INFO_VERSION_MAX
> >          REG_L a0, FW_DYNAMIC_INFO_VERSION_OFFSET(a2)
> > bgt a0, a1, _bad_dynamic_info
> >
> > /* Read boot HART id */
> > li a1, FW_DYNAMIC_INFO_VERSION_2
> > blt a0, a1, 2f
> >          REG_L a0, FW_DYNAMIC_INFO_BOOT_HART_OFFSET(a2)
> > ret
> > 2: li a0, -1
> > ret
>
>
>

[-- Attachment #2: Type: text/html, Size: 6678 bytes --]

Re: Question about RISC-V brom register a1 set value

[Alistair Francis]

On Sat, Oct 1, 2022 at 11:33 AM Eric Chan <e14002270@gmail.com> wrote:
>
> Hi, qemu
>
> As I know, brom will pass 3 parameters to the next stage bootloader, ex: openSBI.
> a0 will pass hartid, a2 will pass fw_dynamic_info start address.
> although a1 doesn't use directly in openSBI.
> a1 read value is determined in compile time rather than read from the original a1 that passes from brom.
> In qemu/hw/riscv/boot.c
> both 32bit and 64bit machines read 4byte that offset 32byte from the brom start address.
>
> for 64 bits machine: a1 read low 32bit data member magic of fw_dynamic_info,

Hello Eric,

a2 will contain the address of the fw_dynamic_info

> the value will same as FW_DYNAMIC_INFO_MAGIC_VALUE because risc-v is little endian.
>
> for 32bits machine: each data member of fw_dynamic_info is 4 bytes, so a1 will read the version rather than magic.

On 32-bit RISC-V a2 will also contain the address of fw_dynamic_info

>
> Do the 32bit and 64bit pass different parameters are expected?

I'm not clear what the difference between the 32-bit and 64-bit
variants are. We use different load instructions, but otherwise they
should be the same functionality.

> If it is not expected, I guess the original version is 64bit machine, and then supports 32bit but misses this detail, I hope I can have an opportunity to fix this problem.
> If it is expected, why they must be done?

It might be easier to send a patch with any fixes you think should be
made. That way it's easy to understand what you think needs to be
fixed.

Alistair

>
> Thanks,
> Eric Chan
>
> qemu/include/hw/riscv/boot_opensbi.h
> #define FW_DYNAMIC_INFO_MAGIC_VALUE     0x4942534f
> qemu/hw/riscv/boot.c
> void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts,
>                                hwaddr start_addr,
>                                hwaddr rom_base, hwaddr rom_size,
>                                uint64_t kernel_entry,
>                                uint64_t fdt_load_addr)
> {
>     int i;
>     uint32_t start_addr_hi32 = 0x00000000;
>     uint32_t fdt_load_addr_hi32 = 0x00000000;
>
>     if (!riscv_is_32bit(harts)) {
>         start_addr_hi32 = start_addr >> 32;
>         fdt_load_addr_hi32 = fdt_load_addr >> 32;
>     }
>     /* reset vector */
>     uint32_t reset_vec[10] = {
>         0x00000297,                  /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
>         0x02828613,                  /*     addi   a2, t0, %pcrel_lo(1b) */
>         0xf1402573,                  /*     csrr   a0, mhartid  */
>         0,
>         0,
>         0x00028067,                  /*     jr     t0 */
>         start_addr,                  /* start: .dword */
>         start_addr_hi32,
>         fdt_load_addr,               /* fdt_laddr: .dword */
>         fdt_load_addr_hi32,
>                                      /* fw_dyn: */
>     };
>     if (riscv_is_32bit(harts)) {
>         reset_vec[3] = 0x0202a583;   /*     lw     a1, 32(t0) */
>         reset_vec[4] = 0x0182a283;   /*     lw     t0, 24(t0) */
>     } else {
>         reset_vec[3] = 0x0202b583;   /*     ld     a1, 32(t0) */
>         reset_vec[4] = 0x0182b283;   /*     ld     t0, 24(t0) */
>     }
>
>     /* copy in the reset vector in little_endian byte order */
>     for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
>         reset_vec[i] = cpu_to_le32(reset_vec[i]);
>     }
>     rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
>                           rom_base, &address_space_memory);
>     riscv_rom_copy_firmware_info(machine, rom_base, rom_size, sizeof(reset_vec),
>                                  kernel_entry);
> }
>
> opensbi/firmware/fw_dynamic.S
> fw_boot_hart:
>         /* Sanity checks */
>         li      a1, FW_DYNAMIC_INFO_MAGIC_VALUE
>         REG_L   a0, FW_DYNAMIC_INFO_MAGIC_OFFSET(a2)
>         bne     a0, a1, _bad_dynamic_info
>         li      a1, FW_DYNAMIC_INFO_VERSION_MAX
>         REG_L   a0, FW_DYNAMIC_INFO_VERSION_OFFSET(a2)
>         bgt     a0, a1, _bad_dynamic_info
>
>         /* Read boot HART id */
>         li      a1, FW_DYNAMIC_INFO_VERSION_2
>         blt     a0, a1, 2f
>         REG_L   a0, FW_DYNAMIC_INFO_BOOT_HART_OFFSET(a2)
>         ret
> 2:      li      a0, -1
>         ret