| Message ID | 20200901154711.18457-1-green.wan@sifive.com |
|---|---|
| Headers | show |
| Series | Add file-backed and write-once features to OTP | expand |
On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > Add '-drive' support to OTP device. Allow users to assign a raw file > as OTP image. Do you mind writing an example command line argument in the commit message? Also, do you have a test case for this? I would like to add it to my CI. > > Signed-off-by: Green Wan <green.wan@sifive.com> > --- > hw/riscv/sifive_u_otp.c | 50 +++++++++++++++++++++++++++++++++ > include/hw/riscv/sifive_u_otp.h | 2 ++ > 2 files changed, 52 insertions(+) > > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c > index b8369e9035..477c54c7b8 100644 > --- a/hw/riscv/sifive_u_otp.c > +++ b/hw/riscv/sifive_u_otp.c > @@ -24,6 +24,8 @@ > #include "qemu/log.h" > #include "qemu/module.h" > #include "hw/riscv/sifive_u_otp.h" > +#include "sysemu/blockdev.h" > +#include "sysemu/block-backend.h" > > #define WRITTEN_BIT_ON 0x1 > > @@ -54,6 +56,16 @@ 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)) { > + > + /* read from backend */ > + if (s->blk) { > + int32_t buf; > + > + blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf, > + SIFIVE_U_OTP_FUSE_WORD); > + return buf; > + } > + > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK]; > } else { > return 0xff; > @@ -145,6 +157,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr, > /* write bit data */ > SET_FUSEARRAY_BIT(s->fuse, s->pa, s->paio, s->pdin); > > + /* write to backend */ > + if (s->blk) { > + blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &val32, > + SIFIVE_U_OTP_FUSE_WORD, 0); > + } > + > /* update written bit */ > SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON); > } > @@ -168,16 +186,48 @@ static const MemoryRegionOps sifive_u_otp_ops = { > > static Property sifive_u_otp_properties[] = { > DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0), > + DEFINE_PROP_DRIVE("drive", SiFiveUOTPState, blk), > DEFINE_PROP_END_OF_LIST(), > }; > > static void sifive_u_otp_realize(DeviceState *dev, Error **errp) > { > SiFiveUOTPState *s = SIFIVE_U_OTP(dev); > + DriveInfo *dinfo; > > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_u_otp_ops, s, > TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE); > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > + > + dinfo = drive_get_next(IF_NONE); > + if (dinfo) { > + int ret; > + uint64_t perm; > + int filesize; > + BlockBackend *blk; > + > + blk = blk_by_legacy_dinfo(dinfo); I think this should be: blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; > + filesize = SIFIVE_U_OTP_NUM_FUSES * SIFIVE_U_OTP_FUSE_WORD; > + if (blk_getlength(blk) < filesize) { > + qemu_log_mask(LOG_GUEST_ERROR, "OTP drive size < 16K\n"); You should probably be setting errp instead. If a user specified a -drive argument, they probably want to error if we aren't going to use it. > + return; > + } > + > + qdev_prop_set_drive(dev, "drive", blk); > + > + perm = BLK_PERM_CONSISTENT_READ | > + (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE); > + ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp); > + if (ret < 0) { > + qemu_log_mask(LOG_GUEST_ERROR, "set perm error."); Is it worth printing the error? > + } > + > + if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "failed to read the initial flash content"); > + return; You don't need a return here. Is this error fatal? Alistair > + } > + } > } > > static void sifive_u_otp_reset(DeviceState *dev) > diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h > index 4a5a0acf48..9bc781fd4f 100644 > --- a/include/hw/riscv/sifive_u_otp.h > +++ b/include/hw/riscv/sifive_u_otp.h > @@ -45,6 +45,7 @@ > > #define SIFIVE_U_OTP_PA_MASK 0xfff > #define SIFIVE_U_OTP_NUM_FUSES 0x1000 > +#define SIFIVE_U_OTP_FUSE_WORD 4 > #define SIFIVE_U_OTP_SERIAL_ADDR 0xfc > > #define SIFIVE_U_OTP_REG_SIZE 0x1000 > @@ -78,6 +79,7 @@ typedef struct SiFiveUOTPState { > uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES]; > /* config */ > uint32_t serial; > + BlockBackend *blk; > } SiFiveUOTPState; > > #endif /* HW_SIFIVE_U_OTP_H */ > -- > 2.17.1 > >
Hi Alistair, Thanks for the review. See the reply inline below. On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > as OTP image. > > Do you mind writing an example command line argument in the commit message? > > Also, do you have a test case for this? I would like to add it to my CI. > Do you mean qtest? I run uboot and use uboot driver to test it and didn't create a qemu test case. Here is the command I use: $ qemu-system-riscv64 -M sifive_u -m 256M -nographic -bios none -kernel ../opensbi/build/platform/sifive/fu540/firmware/fw_payload.elf -d guest_errors -drive if=none,format=raw,file=otp.img I'll check how to create a test case but maybe not that soon in the next patch. > > > > Signed-off-by: Green Wan <green.wan@sifive.com> > > --- > > hw/riscv/sifive_u_otp.c | 50 +++++++++++++++++++++++++++++++++ > > include/hw/riscv/sifive_u_otp.h | 2 ++ > > 2 files changed, 52 insertions(+) > > > > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c > > index b8369e9035..477c54c7b8 100644 > > --- a/hw/riscv/sifive_u_otp.c > > +++ b/hw/riscv/sifive_u_otp.c > > @@ -24,6 +24,8 @@ > > #include "qemu/log.h" > > #include "qemu/module.h" > > #include "hw/riscv/sifive_u_otp.h" > > +#include "sysemu/blockdev.h" > > +#include "sysemu/block-backend.h" > > > > #define WRITTEN_BIT_ON 0x1 > > > > @@ -54,6 +56,16 @@ 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)) { > > + > > + /* read from backend */ > > + if (s->blk) { > > + int32_t buf; > > + > > + blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf, > > + SIFIVE_U_OTP_FUSE_WORD); > > + return buf; > > + } > > + > > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK]; > > } else { > > return 0xff; > > @@ -145,6 +157,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr, > > /* write bit data */ > > SET_FUSEARRAY_BIT(s->fuse, s->pa, s->paio, s->pdin); > > > > + /* write to backend */ > > + if (s->blk) { > > + blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &val32, > > + SIFIVE_U_OTP_FUSE_WORD, 0); > > + } > > + > > /* update written bit */ > > SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON); > > } > > @@ -168,16 +186,48 @@ static const MemoryRegionOps sifive_u_otp_ops = { > > > > static Property sifive_u_otp_properties[] = { > > DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0), > > + DEFINE_PROP_DRIVE("drive", SiFiveUOTPState, blk), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > > static void sifive_u_otp_realize(DeviceState *dev, Error **errp) > > { > > SiFiveUOTPState *s = SIFIVE_U_OTP(dev); > > + DriveInfo *dinfo; > > > > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_u_otp_ops, s, > > TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE); > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > > + > > + dinfo = drive_get_next(IF_NONE); > > + if (dinfo) { > > + int ret; > > + uint64_t perm; > > + int filesize; > > + BlockBackend *blk; > > + > > + blk = blk_by_legacy_dinfo(dinfo); > > I think this should be: > > blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; > The previous code, "if (dinfo)", should check NULL already. But we can add more checks for blk such as qdev_prop_set_drive_err(). > > + filesize = SIFIVE_U_OTP_NUM_FUSES * SIFIVE_U_OTP_FUSE_WORD; > > + if (blk_getlength(blk) < filesize) { > > + qemu_log_mask(LOG_GUEST_ERROR, "OTP drive size < 16K\n"); > > You should probably be setting errp instead. > > If a user specified a -drive argument, they probably want to error if > we aren't going to use it. > Will set an errp. > > + return; > > + } > > + > > + qdev_prop_set_drive(dev, "drive", blk); > > + > > + perm = BLK_PERM_CONSISTENT_READ | > > + (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE); > > + ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp); > > + if (ret < 0) { > > + qemu_log_mask(LOG_GUEST_ERROR, "set perm error."); > > Is it worth printing the error? > Probably add it when I test it. Will remove it. > > + } > > + > > + if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "failed to read the initial flash content"); > > + return; > > You don't need a return here. > k, will remove it and set errp. > Is this error fatal? > This shouldn't be fatal but it might lead to unknown state if the content is read partially. But the checking, "filesize < 16K", is fatal. It leads qemu to abort. > Alistair > > > + } > > + } > > } > > > > static void sifive_u_otp_reset(DeviceState *dev) > > diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h > > index 4a5a0acf48..9bc781fd4f 100644 > > --- a/include/hw/riscv/sifive_u_otp.h > > +++ b/include/hw/riscv/sifive_u_otp.h > > @@ -45,6 +45,7 @@ > > > > #define SIFIVE_U_OTP_PA_MASK 0xfff > > #define SIFIVE_U_OTP_NUM_FUSES 0x1000 > > +#define SIFIVE_U_OTP_FUSE_WORD 4 > > #define SIFIVE_U_OTP_SERIAL_ADDR 0xfc > > > > #define SIFIVE_U_OTP_REG_SIZE 0x1000 > > @@ -78,6 +79,7 @@ typedef struct SiFiveUOTPState { > > uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES]; > > /* config */ > > uint32_t serial; > > + BlockBackend *blk; > > } SiFiveUOTPState; > > > > #endif /* HW_SIFIVE_U_OTP_H */ > > -- > > 2.17.1 > > > >
On Mon, Sep 28, 2020 at 2:18 AM Green Wan <green.wan@sifive.com> wrote: > > Hi Alistair, > > Thanks for the review. See the reply inline below. > > > On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > > as OTP image. > > > > Do you mind writing an example command line argument in the commit message? > > > > Also, do you have a test case for this? I would like to add it to my CI. > > > > Do you mean qtest? I run uboot and use uboot driver to test it and > didn't create a qemu test case. No, I just mean how are you running and testing this. So you are booting U-Boot, then how do you test it in U-Boot? > Here is the command I use: > > $ qemu-system-riscv64 -M sifive_u -m 256M -nographic -bios none > -kernel ../opensbi/build/platform/sifive/fu540/firmware/fw_payload.elf > -d guest_errors -drive if=none,format=raw,file=otp.img > > I'll check how to create a test case but maybe not that soon in the next patch. > > > > > > > Signed-off-by: Green Wan <green.wan@sifive.com> > > > --- > > > hw/riscv/sifive_u_otp.c | 50 +++++++++++++++++++++++++++++++++ > > > include/hw/riscv/sifive_u_otp.h | 2 ++ > > > 2 files changed, 52 insertions(+) > > > > > > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c > > > index b8369e9035..477c54c7b8 100644 > > > --- a/hw/riscv/sifive_u_otp.c > > > +++ b/hw/riscv/sifive_u_otp.c > > > @@ -24,6 +24,8 @@ > > > #include "qemu/log.h" > > > #include "qemu/module.h" > > > #include "hw/riscv/sifive_u_otp.h" > > > +#include "sysemu/blockdev.h" > > > +#include "sysemu/block-backend.h" > > > > > > #define WRITTEN_BIT_ON 0x1 > > > > > > @@ -54,6 +56,16 @@ 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)) { > > > + > > > + /* read from backend */ > > > + if (s->blk) { > > > + int32_t buf; > > > + > > > + blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf, > > > + SIFIVE_U_OTP_FUSE_WORD); > > > + return buf; > > > + } > > > + > > > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK]; > > > } else { > > > return 0xff; > > > @@ -145,6 +157,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr, > > > /* write bit data */ > > > SET_FUSEARRAY_BIT(s->fuse, s->pa, s->paio, s->pdin); > > > > > > + /* write to backend */ > > > + if (s->blk) { > > > + blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &val32, > > > + SIFIVE_U_OTP_FUSE_WORD, 0); > > > + } > > > + > > > /* update written bit */ > > > SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON); > > > } > > > @@ -168,16 +186,48 @@ static const MemoryRegionOps sifive_u_otp_ops = { > > > > > > static Property sifive_u_otp_properties[] = { > > > DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0), > > > + DEFINE_PROP_DRIVE("drive", SiFiveUOTPState, blk), > > > DEFINE_PROP_END_OF_LIST(), > > > }; > > > > > > static void sifive_u_otp_realize(DeviceState *dev, Error **errp) > > > { > > > SiFiveUOTPState *s = SIFIVE_U_OTP(dev); > > > + DriveInfo *dinfo; > > > > > > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_u_otp_ops, s, > > > TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE); > > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > > > + > > > + dinfo = drive_get_next(IF_NONE); > > > + if (dinfo) { > > > + int ret; > > > + uint64_t perm; > > > + int filesize; > > > + BlockBackend *blk; > > > + > > > + blk = blk_by_legacy_dinfo(dinfo); > > > > I think this should be: > > > > blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; > > > > The previous code, "if (dinfo)", should check NULL already. But we can > add more checks for blk such as qdev_prop_set_drive_err(). You are right, but I don't see a fallback if !dinfo > > > > + filesize = SIFIVE_U_OTP_NUM_FUSES * SIFIVE_U_OTP_FUSE_WORD; > > > + if (blk_getlength(blk) < filesize) { > > > + qemu_log_mask(LOG_GUEST_ERROR, "OTP drive size < 16K\n"); > > > > You should probably be setting errp instead. > > > > If a user specified a -drive argument, they probably want to error if > > we aren't going to use it. > > > > Will set an errp. Great! > > > > + return; > > > + } > > > + > > > + qdev_prop_set_drive(dev, "drive", blk); > > > + > > > + perm = BLK_PERM_CONSISTENT_READ | > > > + (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE); > > > + ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp); > > > + if (ret < 0) { > > > + qemu_log_mask(LOG_GUEST_ERROR, "set perm error."); > > > > Is it worth printing the error? > > > Probably add it when I test it. Will remove it. Thanks Alistair > > > > + } > > > + > > > + if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) { > > > + qemu_log_mask(LOG_GUEST_ERROR, > > > + "failed to read the initial flash content"); > > > + return; > > > > You don't need a return here. > > > k, will remove it and set errp. > > > Is this error fatal? > > > This shouldn't be fatal but it might lead to unknown state if the > content is read partially. > But the checking, "filesize < 16K", is fatal. It leads qemu to abort. > > > > Alistair > > > > > + } > > > + } > > > } > > > > > > static void sifive_u_otp_reset(DeviceState *dev) > > > diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h > > > index 4a5a0acf48..9bc781fd4f 100644 > > > --- a/include/hw/riscv/sifive_u_otp.h > > > +++ b/include/hw/riscv/sifive_u_otp.h > > > @@ -45,6 +45,7 @@ > > > > > > #define SIFIVE_U_OTP_PA_MASK 0xfff > > > #define SIFIVE_U_OTP_NUM_FUSES 0x1000 > > > +#define SIFIVE_U_OTP_FUSE_WORD 4 > > > #define SIFIVE_U_OTP_SERIAL_ADDR 0xfc > > > > > > #define SIFIVE_U_OTP_REG_SIZE 0x1000 > > > @@ -78,6 +79,7 @@ typedef struct SiFiveUOTPState { > > > uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES]; > > > /* config */ > > > uint32_t serial; > > > + BlockBackend *blk; > > > } SiFiveUOTPState; > > > > > > #endif /* HW_SIFIVE_U_OTP_H */ > > > -- > > > 2.17.1 > > > > > >
On Wed, Sep 30, 2020 at 1:08 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Mon, Sep 28, 2020 at 2:18 AM Green Wan <green.wan@sifive.com> wrote: > > > > Hi Alistair, > > > > Thanks for the review. See the reply inline below. > > > > > > On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > > > as OTP image. > > > > > > Do you mind writing an example command line argument in the commit message? > > > > > > Also, do you have a test case for this? I would like to add it to my CI. > > > > > > > Do you mean qtest? I run uboot and use uboot driver to test it and > > didn't create a qemu test case. > > No, I just mean how are you running and testing this. > > So you are booting U-Boot, then how do you test it in U-Boot? Correct, I just enabled the configuration for ./drivers/misc/sifive-otp.c in uboot for normal booting access to OTP. And manually modify some failures write case to test write-once feature. > > > Here is the command I use: > > > > $ qemu-system-riscv64 -M sifive_u -m 256M -nographic -bios none > > -kernel ../opensbi/build/platform/sifive/fu540/firmware/fw_payload.elf > > -d guest_errors -drive if=none,format=raw,file=otp.img > > > > I'll check how to create a test case but maybe not that soon in the next patch. > > > > > > > > > > Signed-off-by: Green Wan <green.wan@sifive.com> > > > > --- > > > > hw/riscv/sifive_u_otp.c | 50 +++++++++++++++++++++++++++++++++ > > > > include/hw/riscv/sifive_u_otp.h | 2 ++ > > > > 2 files changed, 52 insertions(+) > > > > > > > > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c > > > > index b8369e9035..477c54c7b8 100644 > > > > --- a/hw/riscv/sifive_u_otp.c > > > > +++ b/hw/riscv/sifive_u_otp.c > > > > @@ -24,6 +24,8 @@ > > > > #include "qemu/log.h" > > > > #include "qemu/module.h" > > > > #include "hw/riscv/sifive_u_otp.h" > > > > +#include "sysemu/blockdev.h" > > > > +#include "sysemu/block-backend.h" > > > > > > > > #define WRITTEN_BIT_ON 0x1 > > > > > > > > @@ -54,6 +56,16 @@ 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)) { > > > > + > > > > + /* read from backend */ > > > > + if (s->blk) { > > > > + int32_t buf; > > > > + > > > > + blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf, > > > > + SIFIVE_U_OTP_FUSE_WORD); > > > > + return buf; > > > > + } > > > > + > > > > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK]; > > > > } else { > > > > return 0xff; > > > > @@ -145,6 +157,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr, > > > > /* write bit data */ > > > > SET_FUSEARRAY_BIT(s->fuse, s->pa, s->paio, s->pdin); > > > > > > > > + /* write to backend */ > > > > + if (s->blk) { > > > > + blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &val32, > > > > + SIFIVE_U_OTP_FUSE_WORD, 0); > > > > + } > > > > + > > > > /* update written bit */ > > > > SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON); > > > > } > > > > @@ -168,16 +186,48 @@ static const MemoryRegionOps sifive_u_otp_ops = { > > > > > > > > static Property sifive_u_otp_properties[] = { > > > > DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0), > > > > + DEFINE_PROP_DRIVE("drive", SiFiveUOTPState, blk), > > > > DEFINE_PROP_END_OF_LIST(), > > > > }; > > > > > > > > static void sifive_u_otp_realize(DeviceState *dev, Error **errp) > > > > { > > > > SiFiveUOTPState *s = SIFIVE_U_OTP(dev); > > > > + DriveInfo *dinfo; > > > > > > > > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_u_otp_ops, s, > > > > TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE); > > > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > > > > + > > > > + dinfo = drive_get_next(IF_NONE); > > > > + if (dinfo) { > > > > + int ret; > > > > + uint64_t perm; > > > > + int filesize; > > > > + BlockBackend *blk; > > > > + > > > > + blk = blk_by_legacy_dinfo(dinfo); > > > > > > I think this should be: > > > > > > blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; > > > > > > > The previous code, "if (dinfo)", should check NULL already. But we can > > add more checks for blk such as qdev_prop_set_drive_err(). > > You are right, but I don't see a fallback if !dinfo > Not sure whether we need it. Originally, I also added '!dinfo' check logic and turned out I found it can be handled like without "-drive" case. It just does a fallback to use the fuse[] array in memory. For example, "qemu-system-riscv64 -M sifive_u -m 256M -nographic -bios none -kernel ../opensbi/build/platform/sifive/fu540/firmware/fw_payload.elf" > > > > > > + filesize = SIFIVE_U_OTP_NUM_FUSES * SIFIVE_U_OTP_FUSE_WORD; > > > > + if (blk_getlength(blk) < filesize) { > > > > + qemu_log_mask(LOG_GUEST_ERROR, "OTP drive size < 16K\n"); > > > > > > You should probably be setting errp instead. > > > > > > If a user specified a -drive argument, they probably want to error if > > > we aren't going to use it. > > > > > > > Will set an errp. > > Great! > > > > > > > + return; > > > > + } > > > > + > > > > + qdev_prop_set_drive(dev, "drive", blk); > > > > + > > > > + perm = BLK_PERM_CONSISTENT_READ | > > > > + (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE); > > > > + ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp); > > > > + if (ret < 0) { > > > > + qemu_log_mask(LOG_GUEST_ERROR, "set perm error."); > > > > > > Is it worth printing the error? > > > > > Probably add it when I test it. Will remove it. > > Thanks > > Alistair > > > > > > > + } > > > > + > > > > + if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) { > > > > + qemu_log_mask(LOG_GUEST_ERROR, > > > > + "failed to read the initial flash content"); > > > > + return; > > > > > > You don't need a return here. > > > > > k, will remove it and set errp. > > > > > Is this error fatal? > > > > > This shouldn't be fatal but it might lead to unknown state if the > > content is read partially. > > But the checking, "filesize < 16K", is fatal. It leads qemu to abort. > > > > > > > Alistair > > > > > > > + } > > > > + } > > > > } > > > > > > > > static void sifive_u_otp_reset(DeviceState *dev) > > > > diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h > > > > index 4a5a0acf48..9bc781fd4f 100644 > > > > --- a/include/hw/riscv/sifive_u_otp.h > > > > +++ b/include/hw/riscv/sifive_u_otp.h > > > > @@ -45,6 +45,7 @@ > > > > > > > > #define SIFIVE_U_OTP_PA_MASK 0xfff > > > > #define SIFIVE_U_OTP_NUM_FUSES 0x1000 > > > > +#define SIFIVE_U_OTP_FUSE_WORD 4 > > > > #define SIFIVE_U_OTP_SERIAL_ADDR 0xfc > > > > > > > > #define SIFIVE_U_OTP_REG_SIZE 0x1000 > > > > @@ -78,6 +79,7 @@ typedef struct SiFiveUOTPState { > > > > uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES]; > > > > /* config */ > > > > uint32_t serial; > > > > + BlockBackend *blk; > > > > } SiFiveUOTPState; > > > > > > > > #endif /* HW_SIFIVE_U_OTP_H */ > > > > -- > > > > 2.17.1 > > > > > > > >
On Wed, Sep 30, 2020 at 12:10 AM Green Wan <green.wan@sifive.com> wrote: > > On Wed, Sep 30, 2020 at 1:08 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Mon, Sep 28, 2020 at 2:18 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > Hi Alistair, > > > > > > Thanks for the review. See the reply inline below. > > > > > > > > > On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > > > > as OTP image. > > > > > > > > Do you mind writing an example command line argument in the commit message? > > > > > > > > Also, do you have a test case for this? I would like to add it to my CI. > > > > > > > > > > Do you mean qtest? I run uboot and use uboot driver to test it and > > > didn't create a qemu test case. > > > > No, I just mean how are you running and testing this. > > > > So you are booting U-Boot, then how do you test it in U-Boot? Hey, Sorry, this email didn't send and I only just noticed. > > Correct, I just enabled the configuration for > ./drivers/misc/sifive-otp.c in uboot for normal booting access to OTP. > And manually modify some failures write case to test write-once > feature. Can you document this? I would like to include this in my tests. Alistair > > > > > > Here is the command I use: > > > > > > $ qemu-system-riscv64 -M sifive_u -m 256M -nographic -bios none > > > -kernel ../opensbi/build/platform/sifive/fu540/firmware/fw_payload.elf > > > -d guest_errors -drive if=none,format=raw,file=otp.img > > > > > > I'll check how to create a test case but maybe not that soon in the next patch. > > > > > > > > > > > > > Signed-off-by: Green Wan <green.wan@sifive.com> > > > > > --- > > > > > hw/riscv/sifive_u_otp.c | 50 +++++++++++++++++++++++++++++++++ > > > > > include/hw/riscv/sifive_u_otp.h | 2 ++ > > > > > 2 files changed, 52 insertions(+) > > > > > > > > > > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c > > > > > index b8369e9035..477c54c7b8 100644 > > > > > --- a/hw/riscv/sifive_u_otp.c > > > > > +++ b/hw/riscv/sifive_u_otp.c > > > > > @@ -24,6 +24,8 @@ > > > > > #include "qemu/log.h" > > > > > #include "qemu/module.h" > > > > > #include "hw/riscv/sifive_u_otp.h" > > > > > +#include "sysemu/blockdev.h" > > > > > +#include "sysemu/block-backend.h" > > > > > > > > > > #define WRITTEN_BIT_ON 0x1 > > > > > > > > > > @@ -54,6 +56,16 @@ 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)) { > > > > > + > > > > > + /* read from backend */ > > > > > + if (s->blk) { > > > > > + int32_t buf; > > > > > + > > > > > + blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf, > > > > > + SIFIVE_U_OTP_FUSE_WORD); > > > > > + return buf; > > > > > + } > > > > > + > > > > > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK]; > > > > > } else { > > > > > return 0xff; > > > > > @@ -145,6 +157,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr, > > > > > /* write bit data */ > > > > > SET_FUSEARRAY_BIT(s->fuse, s->pa, s->paio, s->pdin); > > > > > > > > > > + /* write to backend */ > > > > > + if (s->blk) { > > > > > + blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &val32, > > > > > + SIFIVE_U_OTP_FUSE_WORD, 0); > > > > > + } > > > > > + > > > > > /* update written bit */ > > > > > SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON); > > > > > } > > > > > @@ -168,16 +186,48 @@ static const MemoryRegionOps sifive_u_otp_ops = { > > > > > > > > > > static Property sifive_u_otp_properties[] = { > > > > > DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0), > > > > > + DEFINE_PROP_DRIVE("drive", SiFiveUOTPState, blk), > > > > > DEFINE_PROP_END_OF_LIST(), > > > > > }; > > > > > > > > > > static void sifive_u_otp_realize(DeviceState *dev, Error **errp) > > > > > { > > > > > SiFiveUOTPState *s = SIFIVE_U_OTP(dev); > > > > > + DriveInfo *dinfo; > > > > > > > > > > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_u_otp_ops, s, > > > > > TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE); > > > > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > > > > > + > > > > > + dinfo = drive_get_next(IF_NONE); > > > > > + if (dinfo) { > > > > > + int ret; > > > > > + uint64_t perm; > > > > > + int filesize; > > > > > + BlockBackend *blk; > > > > > + > > > > > + blk = blk_by_legacy_dinfo(dinfo); > > > > > > > > I think this should be: > > > > > > > > blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; > > > > > > > > > > The previous code, "if (dinfo)", should check NULL already. But we can > > > add more checks for blk such as qdev_prop_set_drive_err(). > > > > You are right, but I don't see a fallback if !dinfo > > > Not sure whether we need it. Originally, I also added '!dinfo' check > logic and turned out I found it can be handled like without "-drive" > case. It just does a fallback to use the fuse[] array in memory. For > example, > "qemu-system-riscv64 -M sifive_u -m 256M -nographic -bios none -kernel > ../opensbi/build/platform/sifive/fu540/firmware/fw_payload.elf" > > > > > > > > > > + filesize = SIFIVE_U_OTP_NUM_FUSES * SIFIVE_U_OTP_FUSE_WORD; > > > > > + if (blk_getlength(blk) < filesize) { > > > > > + qemu_log_mask(LOG_GUEST_ERROR, "OTP drive size < 16K\n"); > > > > > > > > You should probably be setting errp instead. > > > > > > > > If a user specified a -drive argument, they probably want to error if > > > > we aren't going to use it. > > > > > > > > > > Will set an errp. > > > > Great! > > > > > > > > > > + return; > > > > > + } > > > > > + > > > > > + qdev_prop_set_drive(dev, "drive", blk); > > > > > + > > > > > + perm = BLK_PERM_CONSISTENT_READ | > > > > > + (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE); > > > > > + ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp); > > > > > + if (ret < 0) { > > > > > + qemu_log_mask(LOG_GUEST_ERROR, "set perm error."); > > > > > > > > Is it worth printing the error? > > > > > > > Probably add it when I test it. Will remove it. > > > > Thanks > > > > Alistair > > > > > > > > > > + } > > > > > + > > > > > + if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) { > > > > > + qemu_log_mask(LOG_GUEST_ERROR, > > > > > + "failed to read the initial flash content"); > > > > > + return; > > > > > > > > You don't need a return here. > > > > > > > k, will remove it and set errp. > > > > > > > Is this error fatal? > > > > > > > This shouldn't be fatal but it might lead to unknown state if the > > > content is read partially. > > > But the checking, "filesize < 16K", is fatal. It leads qemu to abort. > > > > > > > > > > Alistair > > > > > > > > > + } > > > > > + } > > > > > } > > > > > > > > > > static void sifive_u_otp_reset(DeviceState *dev) > > > > > diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h > > > > > index 4a5a0acf48..9bc781fd4f 100644 > > > > > --- a/include/hw/riscv/sifive_u_otp.h > > > > > +++ b/include/hw/riscv/sifive_u_otp.h > > > > > @@ -45,6 +45,7 @@ > > > > > > > > > > #define SIFIVE_U_OTP_PA_MASK 0xfff > > > > > #define SIFIVE_U_OTP_NUM_FUSES 0x1000 > > > > > +#define SIFIVE_U_OTP_FUSE_WORD 4 > > > > > #define SIFIVE_U_OTP_SERIAL_ADDR 0xfc > > > > > > > > > > #define SIFIVE_U_OTP_REG_SIZE 0x1000 > > > > > @@ -78,6 +79,7 @@ typedef struct SiFiveUOTPState { > > > > > uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES]; > > > > > /* config */ > > > > > uint32_t serial; > > > > > + BlockBackend *blk; > > > > > } SiFiveUOTPState; > > > > > > > > > > #endif /* HW_SIFIVE_U_OTP_H */ > > > > > -- > > > > > 2.17.1 > > > > > > > > > >
Hi Alistair, On Wed, Oct 14, 2020 at 10:46 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Wed, Sep 30, 2020 at 12:10 AM Green Wan <green.wan@sifive.com> wrote: > > > > On Wed, Sep 30, 2020 at 1:08 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > On Mon, Sep 28, 2020 at 2:18 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > Hi Alistair, > > > > > > > > Thanks for the review. See the reply inline below. > > > > > > > > > > > > On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > > > > > as OTP image. > > > > > > > > > > Do you mind writing an example command line argument in the commit message? > > > > > > > > > > Also, do you have a test case for this? I would like to add it to my CI. > > > > > > > > > > > > > Do you mean qtest? I run uboot and use uboot driver to test it and > > > > didn't create a qemu test case. > > > > > > No, I just mean how are you running and testing this. > > > > > > So you are booting U-Boot, then how do you test it in U-Boot? > > Hey, > > Sorry, this email didn't send and I only just noticed. > > > > > Correct, I just enabled the configuration for > > ./drivers/misc/sifive-otp.c in uboot for normal booting access to OTP. > > And manually modify some failures write case to test write-once > > feature. > > Can you document this? I would like to include this in my tests. > See `QEMU Specific Instructions` in https://github.com/riscv/opensbi/blob/master/docs/platform/sifive_fu540.md Regards, Bin
On Wed, Oct 14, 2020 at 8:02 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Alistair, > > On Wed, Oct 14, 2020 at 10:46 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Wed, Sep 30, 2020 at 12:10 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > On Wed, Sep 30, 2020 at 1:08 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > On Mon, Sep 28, 2020 at 2:18 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > Hi Alistair, > > > > > > > > > > Thanks for the review. See the reply inline below. > > > > > > > > > > > > > > > On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > > > > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > > > > > > as OTP image. > > > > > > > > > > > > Do you mind writing an example command line argument in the commit message? > > > > > > > > > > > > Also, do you have a test case for this? I would like to add it to my CI. > > > > > > > > > > > > > > > > Do you mean qtest? I run uboot and use uboot driver to test it and > > > > > didn't create a qemu test case. > > > > > > > > No, I just mean how are you running and testing this. > > > > > > > > So you are booting U-Boot, then how do you test it in U-Boot? > > > > Hey, > > > > Sorry, this email didn't send and I only just noticed. > > > > > > > > Correct, I just enabled the configuration for > > > ./drivers/misc/sifive-otp.c in uboot for normal booting access to OTP. > > > And manually modify some failures write case to test write-once > > > feature. > > > > Can you document this? I would like to include this in my tests. > > > > See `QEMU Specific Instructions` in > https://github.com/riscv/opensbi/blob/master/docs/platform/sifive_fu540.md Hmm... I am missing something. I don't see any details on how you access the OTP and verify that reads/writes have occured. That link just seems to document how to build OpenSBI with a U-boot payload. Does U-Boot run the tests automatically after boot? Alistair > > Regards, > Bin
Hi Alistair, That's correct. Once you configure 'CONFIG_SIFIVE_OTP=y', the uboot driver performs some read/write during booting. And I'll also include the test steps by using the 'misc' testing patch shared by Bin. Regards, - Green On Thu, Oct 15, 2020 at 2:51 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Wed, Oct 14, 2020 at 8:02 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Alistair, > > > > On Wed, Oct 14, 2020 at 10:46 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > On Wed, Sep 30, 2020 at 12:10 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > On Wed, Sep 30, 2020 at 1:08 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > > > On Mon, Sep 28, 2020 at 2:18 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > > > Hi Alistair, > > > > > > > > > > > > Thanks for the review. See the reply inline below. > > > > > > > > > > > > > > > > > > On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > > > > > > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > > > > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > > > > > > > as OTP image. > > > > > > > > > > > > > > Do you mind writing an example command line argument in the commit message? > > > > > > > > > > > > > > Also, do you have a test case for this? I would like to add it to my CI. > > > > > > > > > > > > > > > > > > > Do you mean qtest? I run uboot and use uboot driver to test it and > > > > > > didn't create a qemu test case. > > > > > > > > > > No, I just mean how are you running and testing this. > > > > > > > > > > So you are booting U-Boot, then how do you test it in U-Boot? > > > > > > Hey, > > > > > > Sorry, this email didn't send and I only just noticed. > > > > > > > > > > > Correct, I just enabled the configuration for > > > > ./drivers/misc/sifive-otp.c in uboot for normal booting access to OTP. > > > > And manually modify some failures write case to test write-once > > > > feature. > > > > > > Can you document this? I would like to include this in my tests. > > > > > > > See `QEMU Specific Instructions` in > > https://github.com/riscv/opensbi/blob/master/docs/platform/sifive_fu540.md > > Hmm... I am missing something. I don't see any details on how you > access the OTP and verify that reads/writes have occured. That link > just seems to document how to build OpenSBI with a U-boot payload. > > Does U-Boot run the tests automatically after boot? > > Alistair > > > > > Regards, > > Bin
Hi Alistair, On Thu, Oct 15, 2020 at 2:51 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Wed, Oct 14, 2020 at 8:02 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Alistair, > > > > On Wed, Oct 14, 2020 at 10:46 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > On Wed, Sep 30, 2020 at 12:10 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > On Wed, Sep 30, 2020 at 1:08 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > > > On Mon, Sep 28, 2020 at 2:18 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > > > Hi Alistair, > > > > > > > > > > > > Thanks for the review. See the reply inline below. > > > > > > > > > > > > > > > > > > On Sat, Sep 26, 2020 at 5:52 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > > > > > > > > > On Tue, Sep 1, 2020 at 8:49 AM Green Wan <green.wan@sifive.com> wrote: > > > > > > > > > > > > > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > > > > > > > as OTP image. > > > > > > > > > > > > > > Do you mind writing an example command line argument in the commit message? > > > > > > > > > > > > > > Also, do you have a test case for this? I would like to add it to my CI. > > > > > > > > > > > > > > > > > > > Do you mean qtest? I run uboot and use uboot driver to test it and > > > > > > didn't create a qemu test case. > > > > > > > > > > No, I just mean how are you running and testing this. > > > > > > > > > > So you are booting U-Boot, then how do you test it in U-Boot? > > > > > > Hey, > > > > > > Sorry, this email didn't send and I only just noticed. > > > > > > > > > > > Correct, I just enabled the configuration for > > > > ./drivers/misc/sifive-otp.c in uboot for normal booting access to OTP. > > > > And manually modify some failures write case to test write-once > > > > feature. > > > > > > Can you document this? I would like to include this in my tests. > > > > > > > See `QEMU Specific Instructions` in > > https://github.com/riscv/opensbi/blob/master/docs/platform/sifive_fu540.md > > Hmm... I am missing something. I don't see any details on how you > access the OTP and verify that reads/writes have occured. That link > just seems to document how to build OpenSBI with a U-boot payload. > > Does U-Boot run the tests automatically after boot? So far U-Boot only performs the OTP content read during boot, because U-Boot needs to determine its MAC address based on the serial number programmed in the OTP. In normal situations, U-Boot does not perform any write to OTP. That's why I posted a U-Boot patch to test this QEMU functionality of OTP write. The U-Boot patch is a shell command to test the reading / writing of the OTP content. So what you need to do to test this, is to follow the instructions documented in the OpenSBI doc, then use my patch to test the OTP write. Regards, Bin
Based RFC v4 to fix: - Change the patch order - Add write operation to update pdin to fuse[] bit by bit - Fix wrong protection for offset 0x0~0x38 - Add SIFIVE_U_OTP_PWE_EN definition - Refine access macro for fuse[] and fuse_wo[] Summary of Patches - First patch is to add write opertion to update pdin data to fuse[] bit by bit. Add 'write-once' feature to block second write to same bit of the OTP memory. - Second patch is to add file-backed implementation to allow users to use '-drive' to assign an OTP raw image file. OTP image file must be bigger than 16K. For example, '-drive if=none,format=raw,file=otp.img' Testing - Tested on sifive_u for both qemu and u-boot. Green Wan (2): hw/riscv: sifive_u: Add write operation and write-once protection hw/riscv: sifive_u: Add backend drive support hw/riscv/sifive_u_otp.c | 80 ++++++++++++++++++++++++++++++++- include/hw/riscv/sifive_u_otp.h | 5 +++ 2 files changed, 84 insertions(+), 1 deletion(-)