| Message ID | e91f9fccc49a42482d964f380b2ae085de5bfab2.1583285287.git.alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/riscv: Add a serial property to the sifive_u machine | expand |
Hi Alistair, On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis <alistair.francis@wdc.com> wrote: > > At present the board serial number is hard-coded to 1, and passed > to OTP model during initialization. Firmware (FSBL, U-Boot) uses > the serial number to generate a unique MAC address for the on-chip > ethernet controller. When multiple QEMU 'sifive_u' instances are > created and connected to the same subnet, they all have the same > MAC address hence it creates a unusable network. > > A new "serial" property is introduced to the sifive_u SoC to specify > the board serial number. When not given, the default serial number > 1 is used. > > Suggested-by: Bin Meng <bmeng.cn@gmail.com> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/sifive_u.c | 8 +++++++- > include/hw/riscv/sifive_u.h | 2 ++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > index 9a0145b5b4..e52f9d0bd4 100644 > --- a/hw/riscv/sifive_u.c > +++ b/hw/riscv/sifive_u.c > @@ -488,7 +488,7 @@ static void riscv_sifive_u_soc_init(Object *obj) > TYPE_SIFIVE_U_PRCI); > sysbus_init_child_obj(obj, "otp", &s->otp, sizeof(s->otp), > TYPE_SIFIVE_U_OTP); > - qdev_prop_set_uint32(DEVICE(&s->otp), "serial", OTP_SERIAL); > + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial); > sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem), > TYPE_CADENCE_GEM); > } > @@ -607,10 +607,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp) > memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size); > } > > +static Property riscv_sifive_u_soc_props[] = { > + DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL), > + DEFINE_PROP_END_OF_LIST() I am not sure how adding another level of property in the SoC could solve the 'make check' error. > +}; > + > static void riscv_sifive_u_soc_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > > + device_class_set_props(dc, riscv_sifive_u_soc_props); > dc->realize = riscv_sifive_u_soc_realize; > /* Reason: Uses serial_hds in realize function, thus can't be used twice */ > dc->user_creatable = false; > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h > index 82667b5746..a2baa1de5f 100644 > --- a/include/hw/riscv/sifive_u.h > +++ b/include/hw/riscv/sifive_u.h > @@ -42,6 +42,8 @@ typedef struct SiFiveUSoCState { > SiFiveUPRCIState prci; > SiFiveUOTPState otp; > CadenceGEMState gem; > + > + uint32_t serial; > } SiFiveUSoCState; > > #define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u") > -- But anyway this patch does not actually work as expected. See below: $ ./riscv64-softmmu/qemu-system-riscv64 -M sifive_u,serial=3 -nographic -m 2G -bios opensbi_u-boot_sifive_u_64.bin OpenSBI v0.5 (Oct 31 2019 18:38:50) ____ _____ ____ _____ / __ \ / ____| _ \_ _| | | | |_ __ ___ _ __ | (___ | |_) || | | | | | '_ \ / _ \ '_ \ \___ \| _ < | | | |__| | |_) | __/ | | |____) | |_) || |_ \____/| .__/ \___|_| |_|_____/|____/_____| | | |_| Platform Name : SiFive Freedom U540 Platform HART Features : RV64ACDFIMSU Platform Max HARTs : 5 Current Hart : 1 Firmware Base : 0x80000000 Firmware Size : 96 KB Runtime SBI Version : 0.2 PMP0: 0x0000000080000000-0x000000008001ffff (A) PMP1: 0x0000000000000000-0xffffffffffffffff (A,R,W,X) U-Boot 2019.10 (Oct 31 2019 - 18:38:33 +0800) CPU: rv64imafdcsu Model: SiFive HiFive Unleashed A00 DRAM: 2 GiB MMC: In: serial@10010000 Out: serial@10010000 Err: serial@10010000 Net: Warning: ethernet@10090000 MAC addresses don't match: Address in ROM is 52:54:00:12:34:56 Address in environment is 70:b3:d5:92:f0:01 eth0: ethernet@10090000 Hit any key to stop autoboot: 0 See this line: Address in environment is 70:b3:d5:92:f0:01 It should be: 70:b3:d5:92:f0:03 since I specified serial number as 3. Regards, Bin
On Wed, Mar 4, 2020 at 6:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Alistair, > > On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis > <alistair.francis@wdc.com> wrote: > > > > At present the board serial number is hard-coded to 1, and passed > > to OTP model during initialization. Firmware (FSBL, U-Boot) uses > > the serial number to generate a unique MAC address for the on-chip > > ethernet controller. When multiple QEMU 'sifive_u' instances are > > created and connected to the same subnet, they all have the same > > MAC address hence it creates a unusable network. > > > > A new "serial" property is introduced to the sifive_u SoC to specify > > the board serial number. When not given, the default serial number > > 1 is used. > > > > Suggested-by: Bin Meng <bmeng.cn@gmail.com> > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/riscv/sifive_u.c | 8 +++++++- > > include/hw/riscv/sifive_u.h | 2 ++ > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > > index 9a0145b5b4..e52f9d0bd4 100644 > > --- a/hw/riscv/sifive_u.c > > +++ b/hw/riscv/sifive_u.c > > @@ -488,7 +488,7 @@ static void riscv_sifive_u_soc_init(Object *obj) > > TYPE_SIFIVE_U_PRCI); > > sysbus_init_child_obj(obj, "otp", &s->otp, sizeof(s->otp), > > TYPE_SIFIVE_U_OTP); > > - qdev_prop_set_uint32(DEVICE(&s->otp), "serial", OTP_SERIAL); > > + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial); > > sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem), > > TYPE_CADENCE_GEM); > > } > > @@ -607,10 +607,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp) > > memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size); > > } > > > > +static Property riscv_sifive_u_soc_props[] = { > > + DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL), > > + DEFINE_PROP_END_OF_LIST() > > I am not sure how adding another level of property in the SoC could > solve the 'make check' error. The problem is that you were adding a machine property and then you had the SoC reach up to the machine object to get the serial value. This isn't correct and is why the tests fail. This patch series instead adds a property to the machine and the SoC, where the machine sets the SoC property. > > > +}; > > + > > static void riscv_sifive_u_soc_class_init(ObjectClass *oc, void *data) > > { > > DeviceClass *dc = DEVICE_CLASS(oc); > > > > + device_class_set_props(dc, riscv_sifive_u_soc_props); > > dc->realize = riscv_sifive_u_soc_realize; > > /* Reason: Uses serial_hds in realize function, thus can't be used twice */ > > dc->user_creatable = false; > > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h > > index 82667b5746..a2baa1de5f 100644 > > --- a/include/hw/riscv/sifive_u.h > > +++ b/include/hw/riscv/sifive_u.h > > @@ -42,6 +42,8 @@ typedef struct SiFiveUSoCState { > > SiFiveUPRCIState prci; > > SiFiveUOTPState otp; > > CadenceGEMState gem; > > + > > + uint32_t serial; > > } SiFiveUSoCState; > > > > #define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u") > > -- > > But anyway this patch does not actually work as expected. See below: > > $ ./riscv64-softmmu/qemu-system-riscv64 -M sifive_u,serial=3 > -nographic -m 2G -bios opensbi_u-boot_sifive_u_64.bin > > OpenSBI v0.5 (Oct 31 2019 18:38:50) > ____ _____ ____ _____ > / __ \ / ____| _ \_ _| > | | | |_ __ ___ _ __ | (___ | |_) || | > | | | | '_ \ / _ \ '_ \ \___ \| _ < | | > | |__| | |_) | __/ | | |____) | |_) || |_ > \____/| .__/ \___|_| |_|_____/|____/_____| > | | > |_| > > Platform Name : SiFive Freedom U540 > Platform HART Features : RV64ACDFIMSU > Platform Max HARTs : 5 > Current Hart : 1 > Firmware Base : 0x80000000 > Firmware Size : 96 KB > Runtime SBI Version : 0.2 > > PMP0: 0x0000000080000000-0x000000008001ffff (A) > PMP1: 0x0000000000000000-0xffffffffffffffff (A,R,W,X) > > > U-Boot 2019.10 (Oct 31 2019 - 18:38:33 +0800) > > CPU: rv64imafdcsu > Model: SiFive HiFive Unleashed A00 > DRAM: 2 GiB > MMC: > In: serial@10010000 > Out: serial@10010000 > Err: serial@10010000 > Net: > Warning: ethernet@10090000 MAC addresses don't match: > Address in ROM is 52:54:00:12:34:56 > Address in environment is 70:b3:d5:92:f0:01 > eth0: ethernet@10090000 > Hit any key to stop autoboot: 0 > > > See this line: > Address in environment is 70:b3:d5:92:f0:01 > > It should be: 70:b3:d5:92:f0:03 since I specified serial number as 3. Fixed, The problem was setting the serial property too early. Alistair > > Regards, > Bin
Hi Alistair, On Thu, Mar 5, 2020 at 7:13 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Wed, Mar 4, 2020 at 6:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Alistair, > > > > On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis > > <alistair.francis@wdc.com> wrote: > > > > > > At present the board serial number is hard-coded to 1, and passed > > > to OTP model during initialization. Firmware (FSBL, U-Boot) uses > > > the serial number to generate a unique MAC address for the on-chip > > > ethernet controller. When multiple QEMU 'sifive_u' instances are > > > created and connected to the same subnet, they all have the same > > > MAC address hence it creates a unusable network. > > > > > > A new "serial" property is introduced to the sifive_u SoC to specify > > > the board serial number. When not given, the default serial number > > > 1 is used. > > > > > > Suggested-by: Bin Meng <bmeng.cn@gmail.com> > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > > --- > > > hw/riscv/sifive_u.c | 8 +++++++- > > > include/hw/riscv/sifive_u.h | 2 ++ > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > > > index 9a0145b5b4..e52f9d0bd4 100644 > > > --- a/hw/riscv/sifive_u.c > > > +++ b/hw/riscv/sifive_u.c > > > @@ -488,7 +488,7 @@ static void riscv_sifive_u_soc_init(Object *obj) > > > TYPE_SIFIVE_U_PRCI); > > > sysbus_init_child_obj(obj, "otp", &s->otp, sizeof(s->otp), > > > TYPE_SIFIVE_U_OTP); > > > - qdev_prop_set_uint32(DEVICE(&s->otp), "serial", OTP_SERIAL); > > > + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial); > > > sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem), > > > TYPE_CADENCE_GEM); > > > } > > > @@ -607,10 +607,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp) > > > memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size); > > > } > > > > > > +static Property riscv_sifive_u_soc_props[] = { > > > + DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL), > > > + DEFINE_PROP_END_OF_LIST() > > > > I am not sure how adding another level of property in the SoC could > > solve the 'make check' error. > > The problem is that you were adding a machine property and then you > had the SoC reach up to the machine object to get the serial value. > This isn't correct and is why the tests fail. > So looks the failure was due to a check in the test codes only? As I did not see QEMU crashed during my normal usage. > This patch series instead adds a property to the machine and the SoC, > where the machine sets the SoC property. > Regards, Bin
On Thu, Mar 5, 2020 at 1:31 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Alistair, > > On Thu, Mar 5, 2020 at 7:13 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Wed, Mar 4, 2020 at 6:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Alistair, > > > > > > On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis > > > <alistair.francis@wdc.com> wrote: > > > > > > > > At present the board serial number is hard-coded to 1, and passed > > > > to OTP model during initialization. Firmware (FSBL, U-Boot) uses > > > > the serial number to generate a unique MAC address for the on-chip > > > > ethernet controller. When multiple QEMU 'sifive_u' instances are > > > > created and connected to the same subnet, they all have the same > > > > MAC address hence it creates a unusable network. > > > > > > > > A new "serial" property is introduced to the sifive_u SoC to specify > > > > the board serial number. When not given, the default serial number > > > > 1 is used. > > > > > > > > Suggested-by: Bin Meng <bmeng.cn@gmail.com> > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > > > --- > > > > hw/riscv/sifive_u.c | 8 +++++++- > > > > include/hw/riscv/sifive_u.h | 2 ++ > > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > > > > index 9a0145b5b4..e52f9d0bd4 100644 > > > > --- a/hw/riscv/sifive_u.c > > > > +++ b/hw/riscv/sifive_u.c > > > > @@ -488,7 +488,7 @@ static void riscv_sifive_u_soc_init(Object *obj) > > > > TYPE_SIFIVE_U_PRCI); > > > > sysbus_init_child_obj(obj, "otp", &s->otp, sizeof(s->otp), > > > > TYPE_SIFIVE_U_OTP); > > > > - qdev_prop_set_uint32(DEVICE(&s->otp), "serial", OTP_SERIAL); > > > > + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial); > > > > sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem), > > > > TYPE_CADENCE_GEM); > > > > } > > > > @@ -607,10 +607,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp) > > > > memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size); > > > > } > > > > > > > > +static Property riscv_sifive_u_soc_props[] = { > > > > + DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL), > > > > + DEFINE_PROP_END_OF_LIST() > > > > > > I am not sure how adding another level of property in the SoC could > > > solve the 'make check' error. > > > > The problem is that you were adding a machine property and then you > > had the SoC reach up to the machine object to get the serial value. > > This isn't correct and is why the tests fail. > > > > So looks the failure was due to a check in the test codes only? As I > did not see QEMU crashed during my normal usage. No, the bug was in the actual implementation. You were just lucky that you didn't see any issues as in your case you could access the machine state. The make check probably added the SoC individually and hence caught the bug. Alistair > > > This patch series instead adds a property to the machine and the SoC, > > where the machine sets the SoC property. > > > > Regards, > Bin
Hi Alistair, On Fri, Mar 6, 2020 at 12:53 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Thu, Mar 5, 2020 at 1:31 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Alistair, > > > > On Thu, Mar 5, 2020 at 7:13 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > On Wed, Mar 4, 2020 at 6:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Hi Alistair, > > > > > > > > On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis > > > > <alistair.francis@wdc.com> wrote: > > > > > > > > > > At present the board serial number is hard-coded to 1, and passed > > > > > to OTP model during initialization. Firmware (FSBL, U-Boot) uses > > > > > the serial number to generate a unique MAC address for the on-chip > > > > > ethernet controller. When multiple QEMU 'sifive_u' instances are > > > > > created and connected to the same subnet, they all have the same > > > > > MAC address hence it creates a unusable network. > > > > > > > > > > A new "serial" property is introduced to the sifive_u SoC to specify > > > > > the board serial number. When not given, the default serial number > > > > > 1 is used. > > > > > > > > > > Suggested-by: Bin Meng <bmeng.cn@gmail.com> > > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > > > > --- > > > > > hw/riscv/sifive_u.c | 8 +++++++- > > > > > include/hw/riscv/sifive_u.h | 2 ++ > > > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > > > > > index 9a0145b5b4..e52f9d0bd4 100644 > > > > > --- a/hw/riscv/sifive_u.c > > > > > +++ b/hw/riscv/sifive_u.c > > > > > @@ -488,7 +488,7 @@ static void riscv_sifive_u_soc_init(Object *obj) > > > > > TYPE_SIFIVE_U_PRCI); > > > > > sysbus_init_child_obj(obj, "otp", &s->otp, sizeof(s->otp), > > > > > TYPE_SIFIVE_U_OTP); > > > > > - qdev_prop_set_uint32(DEVICE(&s->otp), "serial", OTP_SERIAL); > > > > > + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial); > > > > > sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem), > > > > > TYPE_CADENCE_GEM); > > > > > } > > > > > @@ -607,10 +607,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp) > > > > > memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size); > > > > > } > > > > > > > > > > +static Property riscv_sifive_u_soc_props[] = { > > > > > + DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL), > > > > > + DEFINE_PROP_END_OF_LIST() > > > > > > > > I am not sure how adding another level of property in the SoC could > > > > solve the 'make check' error. > > > > > > The problem is that you were adding a machine property and then you > > > had the SoC reach up to the machine object to get the serial value. > > > This isn't correct and is why the tests fail. > > > > > > > So looks the failure was due to a check in the test codes only? As I > > did not see QEMU crashed during my normal usage. > > No, the bug was in the actual implementation. You were just lucky that > you didn't see any issues as in your case you could access the machine > state. The make check probably added the SoC individually and hence > caught the bug. That sounds like the difference that caused the crash in the test. Thanks for helping this! Regards, Bin
On Thu, Mar 5, 2020 at 4:09 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Alistair, > > On Fri, Mar 6, 2020 at 12:53 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Thu, Mar 5, 2020 at 1:31 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Alistair, > > > > > > On Thu, Mar 5, 2020 at 7:13 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > On Wed, Mar 4, 2020 at 6:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Alistair, > > > > > > > > > > On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis > > > > > <alistair.francis@wdc.com> wrote: > > > > > > > > > > > > At present the board serial number is hard-coded to 1, and passed > > > > > > to OTP model during initialization. Firmware (FSBL, U-Boot) uses > > > > > > the serial number to generate a unique MAC address for the on-chip > > > > > > ethernet controller. When multiple QEMU 'sifive_u' instances are > > > > > > created and connected to the same subnet, they all have the same > > > > > > MAC address hence it creates a unusable network. > > > > > > > > > > > > A new "serial" property is introduced to the sifive_u SoC to specify > > > > > > the board serial number. When not given, the default serial number > > > > > > 1 is used. > > > > > > > > > > > > Suggested-by: Bin Meng <bmeng.cn@gmail.com> > > > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > > > > > --- > > > > > > hw/riscv/sifive_u.c | 8 +++++++- > > > > > > include/hw/riscv/sifive_u.h | 2 ++ > > > > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > > > > > > index 9a0145b5b4..e52f9d0bd4 100644 > > > > > > --- a/hw/riscv/sifive_u.c > > > > > > +++ b/hw/riscv/sifive_u.c > > > > > > @@ -488,7 +488,7 @@ static void riscv_sifive_u_soc_init(Object *obj) > > > > > > TYPE_SIFIVE_U_PRCI); > > > > > > sysbus_init_child_obj(obj, "otp", &s->otp, sizeof(s->otp), > > > > > > TYPE_SIFIVE_U_OTP); > > > > > > - qdev_prop_set_uint32(DEVICE(&s->otp), "serial", OTP_SERIAL); > > > > > > + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial); > > > > > > sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem), > > > > > > TYPE_CADENCE_GEM); > > > > > > } > > > > > > @@ -607,10 +607,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp) > > > > > > memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size); > > > > > > } > > > > > > > > > > > > +static Property riscv_sifive_u_soc_props[] = { > > > > > > + DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL), > > > > > > + DEFINE_PROP_END_OF_LIST() > > > > > > > > > > I am not sure how adding another level of property in the SoC could > > > > > solve the 'make check' error. > > > > > > > > The problem is that you were adding a machine property and then you > > > > had the SoC reach up to the machine object to get the serial value. > > > > This isn't correct and is why the tests fail. > > > > > > > > > > So looks the failure was due to a check in the test codes only? As I > > > did not see QEMU crashed during my normal usage. > > > > No, the bug was in the actual implementation. You were just lucky that > > you didn't see any issues as in your case you could access the machine > > state. The make check probably added the SoC individually and hence > > caught the bug. > > That sounds like the difference that caused the crash in the test. > Thanks for helping this! No worries! Alistair > > Regards, > Bin
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index 9a0145b5b4..e52f9d0bd4 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -488,7 +488,7 @@ static void riscv_sifive_u_soc_init(Object *obj) TYPE_SIFIVE_U_PRCI); sysbus_init_child_obj(obj, "otp", &s->otp, sizeof(s->otp), TYPE_SIFIVE_U_OTP); - qdev_prop_set_uint32(DEVICE(&s->otp), "serial", OTP_SERIAL); + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial); sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem), TYPE_CADENCE_GEM); } @@ -607,10 +607,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp) memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size); } +static Property riscv_sifive_u_soc_props[] = { + DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL), + DEFINE_PROP_END_OF_LIST() +}; + static void riscv_sifive_u_soc_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); + device_class_set_props(dc, riscv_sifive_u_soc_props); dc->realize = riscv_sifive_u_soc_realize; /* Reason: Uses serial_hds in realize function, thus can't be used twice */ dc->user_creatable = false; diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h index 82667b5746..a2baa1de5f 100644 --- a/include/hw/riscv/sifive_u.h +++ b/include/hw/riscv/sifive_u.h @@ -42,6 +42,8 @@ typedef struct SiFiveUSoCState { SiFiveUPRCIState prci; SiFiveUOTPState otp; CadenceGEMState gem; + + uint32_t serial; } SiFiveUSoCState; #define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u")
At present the board serial number is hard-coded to 1, and passed to OTP model during initialization. Firmware (FSBL, U-Boot) uses the serial number to generate a unique MAC address for the on-chip ethernet controller. When multiple QEMU 'sifive_u' instances are created and connected to the same subnet, they all have the same MAC address hence it creates a unusable network. A new "serial" property is introduced to the sifive_u SoC to specify the board serial number. When not given, the default serial number 1 is used. Suggested-by: Bin Meng <bmeng.cn@gmail.com> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- hw/riscv/sifive_u.c | 8 +++++++- include/hw/riscv/sifive_u.h | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-)