diff mbox series

[PULL,12/12] hw/misc/sifive_u_otp: Add backend drive support

Message ID 20201023151619.3175155-13-alistair.francis@wdc.com
State New
Headers show
Series riscv-to-apply queue | expand

Commit Message

Alistair Francis Oct. 23, 2020, 3:16 p.m. UTC 
From: Green Wan <green.wan@sifive.com>

Add '-drive' support to OTP device. Allow users to assign a raw file
as OTP image.

test commands for 16k otp.img filled with zero:

$ dd if=/dev/zero of=./otp.img bs=1k count=16
$ ./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

Signed-off-by: Green Wan <green.wan@sifive.com>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20201020033732.12921-3-green.wan@sifive.com
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/misc/sifive_u_otp.h |  2 ++
 hw/misc/sifive_u_otp.c         | 65 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

Comments

Peter Maydell Nov. 2, 2020, 5:49 p.m. UTC | #1 
On Fri, 23 Oct 2020 at 16:27, Alistair Francis <alistair.francis@wdc.com> wrote:
>
> From: Green Wan <green.wan@sifive.com>
>
> Add '-drive' support to OTP device. Allow users to assign a raw file
> as OTP image.

Hi; Coverity reports some issues with this code (CID 1435959,
CID 1435960, CID 1435961). They're all basically the same thing:
in read, write and reset functions this code calls blk_pread()
or blk_pwrite() but doesn't check the return value, so if the
underlying file operation fails then the guest will be
returned garbage data or have its write thrown away without
either the guest or the user being warned about that.

> @@ -54,6 +57,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 +158,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,
> +                           &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD, 0);
> +            }
> +
>              /* update written bit */
>              SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON);
>          }
> @@ -168,16 +187,48 @@ static const MemoryRegionOps sifive_u_otp_ops = {

>  static void sifive_u_otp_reset(DeviceState *dev)
> @@ -191,6 +242,20 @@ static void sifive_u_otp_reset(DeviceState *dev)
>      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
>      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
>
> +    if (s->blk) {
> +        /* Put serial number to backend as well*/
> +        uint32_t serial_data;
> +        int index = SIFIVE_U_OTP_SERIAL_ADDR;
> +
> +        serial_data = s->serial;
> +        blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +
> +        serial_data = ~(s->serial);
> +        blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +    }
> +
>      /* Initialize write-once map */
>      memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));
>  }
> --
> 2.28.0

thanks
-- PMM
Alistair Francis Nov. 3, 2020, 3:41 p.m. UTC | #2 
On Mon, Nov 2, 2020 at 9:49 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 23 Oct 2020 at 16:27, Alistair Francis <alistair.francis@wdc.com> wrote:
> >
> > From: Green Wan <green.wan@sifive.com>
> >
> > Add '-drive' support to OTP device. Allow users to assign a raw file
> > as OTP image.
>
> Hi; Coverity reports some issues with this code (CID 1435959,
> CID 1435960, CID 1435961). They're all basically the same thing:
> in read, write and reset functions this code calls blk_pread()
> or blk_pwrite() but doesn't check the return value, so if the
> underlying file operation fails then the guest will be
> returned garbage data or have its write thrown away without
> either the guest or the user being warned about that.

Green Wan are you able to send a patch to check the error value?

Alistair

>
> > @@ -54,6 +57,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 +158,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,
> > +                           &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD, 0);
> > +            }
> > +
> >              /* update written bit */
> >              SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON);
> >          }
> > @@ -168,16 +187,48 @@ static const MemoryRegionOps sifive_u_otp_ops = {
>
> >  static void sifive_u_otp_reset(DeviceState *dev)
> > @@ -191,6 +242,20 @@ static void sifive_u_otp_reset(DeviceState *dev)
> >      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
> >      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
> >
> > +    if (s->blk) {
> > +        /* Put serial number to backend as well*/
> > +        uint32_t serial_data;
> > +        int index = SIFIVE_U_OTP_SERIAL_ADDR;
> > +
> > +        serial_data = s->serial;
> > +        blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> > +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> > +
> > +        serial_data = ~(s->serial);
> > +        blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> > +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> > +    }
> > +
> >      /* Initialize write-once map */
> >      memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));
> >  }
> > --
> > 2.28.0
>
> thanks
> -- PMM
Green Wan Nov. 4, 2020, 8:19 a.m. UTC | #3 
OK, let me do it. I'll send it soon.

On Tue, Nov 3, 2020 at 11:53 PM Alistair Francis <alistair23@gmail.com> wrote:
>

> On Mon, Nov 2, 2020 at 9:49 AM Peter Maydell <peter.maydell@linaro.org> wrote:

> >

> > On Fri, 23 Oct 2020 at 16:27, Alistair Francis <alistair.francis@wdc.com> wrote:

> > >

> > > From: Green Wan <green.wan@sifive.com>

> > >

> > > Add '-drive' support to OTP device. Allow users to assign a raw file

> > > as OTP image.

> >

> > Hi; Coverity reports some issues with this code (CID 1435959,

> > CID 1435960, CID 1435961). They're all basically the same thing:

> > in read, write and reset functions this code calls blk_pread()

> > or blk_pwrite() but doesn't check the return value, so if the

> > underlying file operation fails then the guest will be

> > returned garbage data or have its write thrown away without

> > either the guest or the user being warned about that.

>

> Green Wan are you able to send a patch to check the error value?

>

> Alistair

>

> >

> > > @@ -54,6 +57,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 +158,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,

> > > +                           &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD, 0);

> > > +            }

> > > +

> > >              /* update written bit */

> > >              SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON);

> > >          }

> > > @@ -168,16 +187,48 @@ static const MemoryRegionOps sifive_u_otp_ops = {

> >

> > >  static void sifive_u_otp_reset(DeviceState *dev)

> > > @@ -191,6 +242,20 @@ static void sifive_u_otp_reset(DeviceState *dev)

> > >      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;

> > >      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);

> > >

> > > +    if (s->blk) {

> > > +        /* Put serial number to backend as well*/

> > > +        uint32_t serial_data;

> > > +        int index = SIFIVE_U_OTP_SERIAL_ADDR;

> > > +

> > > +        serial_data = s->serial;

> > > +        blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,

> > > +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);

> > > +

> > > +        serial_data = ~(s->serial);

> > > +        blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,

> > > +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);

> > > +    }

> > > +

> > >      /* Initialize write-once map */

> > >      memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));

> > >  }

> > > --

> > > 2.28.0

> >

> > thanks

> > -- PMM
diff mbox series

Patch

diff --git a/include/hw/misc/sifive_u_otp.h b/include/hw/misc/sifive_u_otp.h
index ebffbc1fa5..5d0d7df455 100644
--- a/include/hw/misc/sifive_u_otp.h
+++ b/include/hw/misc/sifive_u_otp.h
@@ -46,6 +46,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
@@ -80,6 +81,7 @@  struct SiFiveUOTPState {
     uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
     /* config */
     uint32_t serial;
+    BlockBackend *blk;
 };
 
 #endif /* HW_SIFIVE_U_OTP_H */
diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
index b9238d64cb..60066375ab 100644
--- a/hw/misc/sifive_u_otp.c
+++ b/hw/misc/sifive_u_otp.c
@@ -19,11 +19,14 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "hw/misc/sifive_u_otp.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/block-backend.h"
 
 #define WRITTEN_BIT_ON 0x1
 
@@ -54,6 +57,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 +158,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,
+                           &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD, 0);
+            }
+
             /* update written bit */
             SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON);
         }
@@ -168,16 +187,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) {
+            error_setg(errp, "OTP drive size < 16K");
+            return;
+        }
+
+        qdev_prop_set_drive_err(dev, "drive", blk, errp);
+
+        if (s->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) {
+                return;
+            }
+
+            if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) {
+                error_setg(errp, "failed to read the initial flash content");
+            }
+        }
+    }
 }
 
 static void sifive_u_otp_reset(DeviceState *dev)
@@ -191,6 +242,20 @@  static void sifive_u_otp_reset(DeviceState *dev)
     s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
     s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
 
+    if (s->blk) {
+        /* Put serial number to backend as well*/
+        uint32_t serial_data;
+        int index = SIFIVE_U_OTP_SERIAL_ADDR;
+
+        serial_data = s->serial;
+        blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
+                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
+
+        serial_data = ~(s->serial);
+        blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
+                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
+    }
+
     /* Initialize write-once map */
     memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));
 }