On Wed, Mar 4, 2020 at 3:00 PM Palmer Dabbelt <pal...@dabbelt.com> wrote:
>
> On Sun, 16 Feb 2020 05:55:17 PST (-0800), bmeng...@gmail.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 specify the board serial
> > number. When not given, the default serial number 1 is used.
> >
> > Signed-off-by: Bin Meng <bmeng...@gmail.com>
> >
> > ---
> >
> > Changes in v2:
> > - Move setting OTP serial number property from riscv_sifive_u_soc_init()
> > to riscv_sifive_u_soc_realize(), to fix the 'check-qtest-riscv' error.
> > I am not really sure why doing so could fix the 'make check' error.
> > The v1 patch worked fine and nothing seems wrong.
> >
> > hw/riscv/sifive_u.c | 21 ++++++++++++++++++++-
> > include/hw/riscv/sifive_u.h | 1 +
> > 2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index 0e12b3c..ca561d3 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -34,6 +34,7 @@
> > #include "qemu/log.h"
> > #include "qemu/error-report.h"
> > #include "qapi/error.h"
> > +#include "qapi/visitor.h"
> > #include "hw/boards.h"
> > #include "hw/loader.h"
> > #include "hw/sysbus.h"
> > @@ -434,7 +435,6 @@ 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);
> > sysbus_init_child_obj(obj, "gem", &s->gem, sizeof(s->gem),
> > TYPE_CADENCE_GEM);
> > }
> > @@ -453,6 +453,18 @@ static void sifive_u_set_start_in_flash(Object *obj,
> > bool value, Error **errp)
> > s->start_in_flash = value;
> > }
> >
> > +static void sifive_u_get_serial(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + visit_type_uint32(v, name, (uint32_t *)opaque, errp);
> > +}
> > +
> > +static void sifive_u_set_serial(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + visit_type_uint32(v, name, (uint32_t *)opaque, errp);
> > +}
> > +
> > static void riscv_sifive_u_machine_instance_init(Object *obj)
> > {
> > SiFiveUState *s = RISCV_U_MACHINE(obj);
> > @@ -464,11 +476,17 @@ static void
> > riscv_sifive_u_machine_instance_init(Object *obj)
> > "Set on to tell QEMU's ROM to jump to
> > " \
> > "flash. Otherwise QEMU will jump to
> > DRAM",
> > NULL);
> > +
> > + s->serial = OTP_SERIAL;
> > + object_property_add(obj, "serial", "uint32", sifive_u_get_serial,
> > + sifive_u_set_serial, NULL, &s->serial, NULL);
> > + object_property_set_description(obj, "serial", "Board serial number",
> > NULL);
> > }
> >
> > static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
> > {
> > MachineState *ms = MACHINE(qdev_get_machine());
> > + SiFiveUState *us = RISCV_U_MACHINE(ms);
> > SiFiveUSoCState *s = RISCV_U_SOC(dev);
> > const struct MemmapEntry *memmap = sifive_u_memmap;
> > MemoryRegion *system_memory = get_system_memory();
> > @@ -554,6 +572,7 @@ static void riscv_sifive_u_soc_realize(DeviceState
> > *dev, Error **errp)
> > object_property_set_bool(OBJECT(&s->prci), true, "realized", &err);
> > sysbus_mmio_map(SYS_BUS_DEVICE(&s->prci), 0,
> > memmap[SIFIVE_U_PRCI].base);
> >
> > + qdev_prop_set_uint32(DEVICE(&s->otp), "serial", us->serial);
> > object_property_set_bool(OBJECT(&s->otp), true, "realized", &err);
> > sysbus_mmio_map(SYS_BUS_DEVICE(&s->otp), 0, memmap[SIFIVE_U_OTP].base);
> >
> > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> > index 82667b5..7cf742e 100644
> > --- a/include/hw/riscv/sifive_u.h
> > +++ b/include/hw/riscv/sifive_u.h
> > @@ -59,6 +59,7 @@ typedef struct SiFiveUState {
> > int fdt_size;
> >
> > bool start_in_flash;
> > + uint32_t serial;
> > } SiFiveUState;
> >
> > enum {
>
> Reviewed-by: Palmer Dabbelt <palmerdabb...@google.com>
>
> Thanks. This is in the queue for the soft freeze.
This patch isn't correct. The SoC gets the sifive_u machine state
which is a bad idea. If the SoC is ever user on a different machine
this will crash.
I have sent a patch series that adds a machine and SoC property instead.
Alistair
>
