| Message ID | 20200827074638.21451-1-green.wan@sifive.com |
|---|---|
| Headers | show |
| Series | Add file-backed and write-once features to OTP | expand |
Hi Bin, Thanks for the remindings. I didn't notice the write operation and will add the write functionality based on bit by bit mechanism. Regards, Green On Fri, Aug 28, 2020 at 8:54 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Green, > > On Thu, Aug 27, 2020 at 3:47 PM Green Wan <green.wan@sifive.com> wrote: > > > > Add '-drive' support to OTP device. Allow users to assign a raw file > > as OTP image. > > > > 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 f6ecbaa2ca..aab2220494 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" > > > > static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size) > > { > > @@ -46,6 +48,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; > > @@ -123,6 +135,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr, > > s->ptrim = val32; > > break; > > case SIFIVE_U_OTP_PWE: > > + /* write to backend */ > > + if (s->blk) { > > + blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &val32, > > The logic looks wrong to me. According to the U-Boot driver > (sifive-otp.c) the content to be written to the OTP memory comes from > s->pdin bit by bit. Here val32 represents whether to disable write or > enable write. > > I think we should arrange patches like this: > > patch 1 to add OTP write functionality, to the existing s->fuse[] mechanism > patch 2 to add file based backend write support > > > + SIFIVE_U_OTP_FUSE_WORD, 0); > > + } > > + > > s->pwe = val32; > > break; > > default: > > @@ -143,16 +161,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); > > + 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"); > > + 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."); > > + } > > + > > + if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "failed to read the initial flash content"); > > + return; > > + } > > + } > > } > > > > 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 639297564a..13d2552e43 100644 > > --- a/include/hw/riscv/sifive_u_otp.h > > +++ b/include/hw/riscv/sifive_u_otp.h > > @@ -43,6 +43,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 > > @@ -75,6 +76,7 @@ typedef struct SiFiveUOTPState { > > uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES]; > > /* config */ > > uint32_t serial; > > + BlockBackend *blk; > > } SiFiveUOTPState; > > > > #endif /* HW_SIFIVE_U_OTP_H */ > > -- > > Regards, > Bin
Based RFC v3 to fix: - Fix all nits-related - Use blk for blk_by_legacy_dinfo(dinfo) - Move SET_WRITTEN_BIT out from else branch Summary of Patches - First 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' - Second patch is to add 'write-once' feature to block second write to the OTP memory. Only keep the 'written' state for non-control register range from 0x38 to 16KB. Testing - Tested on sifive_u for both qemu and u-boot. Green Wan (2): hw/riscv: sifive_u: Add backend drive support hw/riscv: sifive_u: Add write-once protection hw/riscv/sifive_u_otp.c | 71 +++++++++++++++++++++++++++++++++ include/hw/riscv/sifive_u_otp.h | 3 ++ 2 files changed, 74 insertions(+)