Hi Bin, Thanks for the reply. I think we can add property to sifive_u_otp_properties[] (something like below) and remove generic code dependency. What do you think of it? @@ -243,6 +245,7 @@ static const MemoryRegionOps sifive_u_otp_ops = { static Property sifive_u_otp_properties[] = { DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0), + DEFINE_PROP_STRING("otp_file", SiFiveUOTPState, otp_file), DEFINE_PROP_END_OF_LIST(), }; typedef struct SiFiveUOTPState { /*< private >*/ SysBusDevice parent_obj; @@ -77,6 +75,7 @@ typedef struct SiFiveUOTPState { uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES]; /* config */ uint32_t serial; + char *otp_file; uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES]; } SiFiveUOTPState; Regards, Green On Fri, Jul 24, 2020 at 10:20 PM Bin Meng wrote: > Hi Green, > > On Fri, Jul 24, 2020 at 5:51 PM Green Wan wrote: > > > > Add a file-backed implementation for OTP of sifive_u machine. Use > > '-boot otp-file=xxx' to enable it. Do file open, mmap and close > > for every OTP read/write in case keep the update-to-date snapshot > > of OTP. > > > > Signed-off-by: Green Wan > > --- > > hw/riscv/sifive_u_otp.c | 88 ++++++++++++++++++++++++++++++++- > > include/hw/riscv/sifive_u_otp.h | 2 + > > qemu-options.hx | 3 +- > > softmmu/vl.c | 6 ++- > > 4 files changed, 96 insertions(+), 3 deletions(-) > > > > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c > > index f6ecbaa2ca..26e1965821 100644 > > --- a/hw/riscv/sifive_u_otp.c > > +++ b/hw/riscv/sifive_u_otp.c > > @@ -24,6 +24,72 @@ > > #include "qemu/log.h" > > #include "qemu/module.h" > > #include "hw/riscv/sifive_u_otp.h" > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define TRACE_PREFIX "FU540_OTP: " > > +#define SIFIVE_FU540_OTP_SIZE (SIFIVE_U_OTP_NUM_FUSES * 4) > > + > > +static int otp_backed_fd; > > +static unsigned int *otp_mmap; > > + > > +static void sifive_u_otp_backed_load(const char *filename); > > +static uint64_t sifive_u_otp_backed_read(uint32_t fuseidx); > > +static void sifive_u_otp_backed_write(uint32_t fuseidx, > > + uint32_t paio, > > + uint32_t pdin); > > +static void sifive_u_otp_backed_unload(void); > > + > > +void sifive_u_otp_backed_load(const char *filename) > > +{ > > + if (otp_backed_fd < 0) { > > + > > + otp_backed_fd = open(filename, O_RDWR); > > + > > + if (otp_backed_fd < 0) > > + qemu_log_mask(LOG_TRACE, > > + TRACE_PREFIX "Warning: can't open otp > file\n"); > > + else { > > + > > + otp_mmap = (unsigned int *)mmap(0, > > + SIFIVE_FU540_OTP_SIZE, > > + PROT_READ | PROT_WRITE | > PROT_EXEC, > > + MAP_FILE | MAP_SHARED, > > + otp_backed_fd, > > + 0); > > + > > + if (otp_mmap == MAP_FAILED) > > + qemu_log_mask(LOG_TRACE, > > + TRACE_PREFIX "Warning: can't mmap otp > file\n"); > > + } > > + } > > + > > +} > > + > > +uint64_t sifive_u_otp_backed_read(uint32_t fuseidx) > > +{ > > + return (uint64_t)(otp_mmap[fuseidx]); > > +} > > + > > +void sifive_u_otp_backed_write(uint32_t fuseidx, uint32_t paio, > uint32_t pdin) > > +{ > > + otp_mmap[fuseidx] &= ~(pdin << paio); > > + otp_mmap[fuseidx] |= (pdin << paio); > > +} > > + > > + > > +void sifive_u_otp_backed_unload(void) > > +{ > > + munmap(otp_mmap, SIFIVE_FU540_OTP_SIZE); > > + close(otp_backed_fd); > > + otp_backed_fd = -1; > > +} > > > > static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned > int size) > > { > > @@ -46,7 +112,17 @@ static uint64_t sifive_u_otp_read(void *opaque, > hwaddr addr, unsigned int size) > > if ((s->pce & SIFIVE_U_OTP_PCE_EN) && > > (s->pdstb & SIFIVE_U_OTP_PDSTB_EN) && > > (s->ptrim & SIFIVE_U_OTP_PTRIM_EN)) { > > - return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK]; > > + > > + if (otp_file) { > > + uint64_t val; > > + > > + sifive_u_otp_backed_load(otp_file); > > + val = sifive_u_otp_backed_read(s->pa); > > + sifive_u_otp_backed_unload(); > > + > > + return val; > > + } else > > + return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK]; > > } else { > > return 0xff; > > } > > @@ -123,6 +199,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr > addr, > > s->ptrim = val32; > > break; > > case SIFIVE_U_OTP_PWE: > > + if (otp_file) { > > + sifive_u_otp_backed_load(otp_file); > > + sifive_u_otp_backed_write(s->pa, s->paio, s->pdin); > > + sifive_u_otp_backed_unload(); > > + } > > + > > s->pwe = val32; > > break; > > default: > > @@ -165,6 +247,10 @@ static void sifive_u_otp_reset(DeviceState *dev) > > /* Make a valid content of serial number */ > > s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial; > > s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial); > > + > > + /* Initialize file mmap and descriptor. */ > > + otp_mmap = NULL; > > + otp_backed_fd = -1; > > } > > > > static void sifive_u_otp_class_init(ObjectClass *klass, void *data) > > diff --git a/include/hw/riscv/sifive_u_otp.h > b/include/hw/riscv/sifive_u_otp.h > > index 639297564a..1342bd7342 100644 > > --- a/include/hw/riscv/sifive_u_otp.h > > +++ b/include/hw/riscv/sifive_u_otp.h > > @@ -52,6 +52,8 @@ > > #define SIFIVE_U_OTP(obj) \ > > OBJECT_CHECK(SiFiveUOTPState, (obj), TYPE_SIFIVE_U_OTP) > > > > +extern const char *otp_file; > > + > > typedef struct SiFiveUOTPState { > > /*< private >*/ > > SysBusDevice parent_obj; > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 708583b4ce..eb9a54f4ed 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -415,10 +415,11 @@ ERST > > > > DEF("boot", HAS_ARG, QEMU_OPTION_boot, > > "-boot [order=drives][,once=drives][,menu=on|off]\n" > > - " > [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n" > > + " > [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off][,otp-file=otp_file]\n" > > " 'drives': floppy (a), hard disk (c), CD-ROM (d), > network (n)\n" > > " 'sp_name': the file's name that would be passed to > bios as logo picture, if menu=on\n" > > " 'sp_time': the period that splash picture last if > menu=on, unit is ms\n" > > + " 'otp_file': the file name backed OTP\n" > > " 'rb_timeout': the timeout before guest reboot when > boot failed, unit is ms\n", > > Instead of touching generic codes, could we add such property at the > board level? > > > QEMU_ARCH_ALL) > > SRST > > Regards, > Bin >