[RFC PATCH v6 0/2] Add file-backed and write-once features to OTP

[RFC PATCH v6 0/2] Add file-backed and write-once features to OTP

[Green Wan]

Changelogs:
v5 to v6:
 - Rebase to latest. (sifive_u_otp.* are moved to hw/misc)
 - Put the example command to commit message.
 - Refine errp handle when check backend drive.
 - Remove unnecessary debug message.

v4 to v5:
 - 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/misc/sifive_u_otp: Add write function and write-once protection
  hw/misc/sifive_u_otp: Add backend drive support

 hw/misc/sifive_u_otp.c         | 81 +++++++++++++++++++++++++++++++++-
 include/hw/misc/sifive_u_otp.h |  5 +++
 2 files changed, 85 insertions(+), 1 deletion(-)

-- 
2.17.1

[RFC PATCH v6 1/2] hw/misc/sifive_u_otp: Add write function and write-once protection

[Green Wan]

 - Add write operation to update fuse data bit when PWE bit is on.
 - Add array, fuse_wo, to store the 'written' status for all bits
   of OTP to block the write operation.

Signed-off-by: Green Wan <green.wan@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/misc/sifive_u_otp.c         | 30 +++++++++++++++++++++++++++++-
 include/hw/misc/sifive_u_otp.h |  3 +++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
index c2f3c8e129..685c1f8e07 100644
--- a/hw/misc/sifive_u_otp.c
+++ b/hw/misc/sifive_u_otp.c
@@ -25,6 +25,14 @@
 #include "qemu/module.h"
 #include "hw/misc/sifive_u_otp.h"
 
+#define WRITTEN_BIT_ON 0x1
+
+#define SET_FUSEARRAY_BIT(map, i, off, bit)    \
+    map[i] = bit ? (map[i] | bit << off) : (map[i] & ~(bit << off))
+
+#define GET_FUSEARRAY_BIT(map, i, off)    \
+    ((map[i] >> off) & 0x1)
+
 static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
 {
     SiFiveUOTPState *s = opaque;
@@ -123,7 +131,24 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
         s->ptrim = val32;
         break;
     case SIFIVE_U_OTP_PWE:
-        s->pwe = val32;
+        s->pwe = val32 & SIFIVE_U_OTP_PWE_EN;
+
+        /* PWE is enabled. Ignore PAS=1 (no redundancy cell) */
+        if (s->pwe && !s->pas) {
+            if (GET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio)) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "Error: write idx<%u>, bit<%u>\n",
+                              s->pa, s->paio);
+                break;
+            }
+
+            /* write bit data */
+            SET_FUSEARRAY_BIT(s->fuse, s->pa, s->paio, s->pdin);
+
+            /* update written bit */
+            SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON);
+        }
+
         break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write: addr=0x%" HWADDR_PRIx
@@ -165,6 +190,9 @@ 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 write-once map */
+    memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));
 }
 
 static void sifive_u_otp_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/misc/sifive_u_otp.h b/include/hw/misc/sifive_u_otp.h
index 82c9176c8f..ebffbc1fa5 100644
--- a/include/hw/misc/sifive_u_otp.h
+++ b/include/hw/misc/sifive_u_otp.h
@@ -36,6 +36,8 @@
 #define SIFIVE_U_OTP_PTRIM      0x34
 #define SIFIVE_U_OTP_PWE        0x38
 
+#define SIFIVE_U_OTP_PWE_EN     (1 << 0)
+
 #define SIFIVE_U_OTP_PCE_EN     (1 << 0)
 
 #define SIFIVE_U_OTP_PDSTB_EN   (1 << 0)
@@ -75,6 +77,7 @@ struct SiFiveUOTPState {
     uint32_t ptrim;
     uint32_t pwe;
     uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
+    uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
     /* config */
     uint32_t serial;
 };
-- 
2.17.1

[RFC PATCH v6 2/2] hw/misc/sifive_u_otp: Add backend drive support

[Green Wan]

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>
---
 hw/misc/sifive_u_otp.c         | 51 ++++++++++++++++++++++++++++++++++
 include/hw/misc/sifive_u_otp.h |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
index 685c1f8e07..f2585c1ed7 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, &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 +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)
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 */
-- 
2.17.1

Re: [RFC PATCH v6 0/2] Add file-backed and write-once features to OTP

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200928101146.12786-1-green.wan@sifive.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

C linker for the host machine: cc ld.bfd 2.27-43
Host machine cpu family: x86_64
Host machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Using expected file 'tests/data/acpi/virt/DSDT.memhp'
socket_accept failed: Resource temporarily unavailable
**
ERROR:../src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
../src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
ERROR qtest-x86_64: bios-tables-test - Bail out! ERROR:../src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
  TEST    iotest-qcow2: 024
make: *** [run-test-138] Error 1
make: *** Waiting for unfinished jobs....
  TEST    iotest-qcow2: 025
  TEST    iotest-qcow2: 027
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=1a4351fb3f90489ebfca763b45f3d5e1', '-u', '1001', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ku3zq5yz/src/docker-src.2020-09-28-07.25.33.19332:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=1a4351fb3f90489ebfca763b45f3d5e1
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ku3zq5yz/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    17m35.301s
user    0m15.921s


The full log is available at
http://patchew.org/logs/20200928101146.12786-1-green.wan@sifive.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [RFC PATCH v6 0/2] Add file-backed and write-once features to OTP

[Bin Meng]

Hi Green,

On Mon, Sep 28, 2020 at 6:11 PM Green Wan <green.wan@sifive.com> wrote:
>
> Changelogs:
> v5 to v6:
>  - Rebase to latest. (sifive_u_otp.* are moved to hw/misc)
>  - Put the example command to commit message.
>  - Refine errp handle when check backend drive.
>  - Remove unnecessary debug message.
>

I will try to test this soon with a modified U-Boot.

Regards,
Bin

Re: [RFC PATCH v6 1/2] hw/misc/sifive_u_otp: Add write function and write-once protection

[Bin Meng]

Hi Green,

On Mon, Sep 28, 2020 at 6:12 PM Green Wan <green.wan@sifive.com> wrote:
>
>  - Add write operation to update fuse data bit when PWE bit is on.
>  - Add array, fuse_wo, to store the 'written' status for all bits
>    of OTP to block the write operation.
>
> Signed-off-by: Green Wan <green.wan@sifive.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/misc/sifive_u_otp.c         | 30 +++++++++++++++++++++++++++++-
>  include/hw/misc/sifive_u_otp.h |  3 +++
>  2 files changed, 32 insertions(+), 1 deletion(-)
>

I am not sure how you tested this. I wrote a simple U-Boot command to
call U-Boot sifive-otp driver to test the write functionality, but it
failed.

=> misc write otp@10070000 0 80200000 10
=> misc read  otp@10070000 0 80400000 10
=> md 80400000
80400000: ffffffff ffffffff ffffffff ffffffff    ................
80400010: 00000000 00000000 00000000 00000000    ................
80400020: 00000000 00000000 00000000 00000000    ................
80400030: 00000000 00000000 00000000 00000000    ................
80400040: 00000000 00000000 00000000 00000000    ................
80400050: 00000000 00000000 00000000 00000000    ................
80400060: 00000000 00000000 00000000 00000000    ................
80400070: 00000000 00000000 00000000 00000000    ................
80400080: 00000000 00000000 00000000 00000000    ................
80400090: 00000000 00000000 00000000 00000000    ................
804000a0: 00000000 00000000 00000000 00000000    ................
804000b0: 00000000 00000000 00000000 00000000    ................
804000c0: 00000000 00000000 00000000 00000000    ................
804000d0: 00000000 00000000 00000000 00000000    ................
804000e0: 00000000 00000000 00000000 00000000    ................
804000f0: 00000000 00000000 00000000 00000000    ................
=> misc write otp@10070000 0 80200010 10
=> misc read  otp@10070000 0 80400010 10
=> md 80400000
80400000: ffffffff ffffffff ffffffff ffffffff    ................
80400010: ffffffff ffffffff ffffffff ffffffff    ................
80400020: 00000000 00000000 00000000 00000000    ................
80400030: 00000000 00000000 00000000 00000000    ................
80400040: 00000000 00000000 00000000 00000000    ................
80400050: 00000000 00000000 00000000 00000000    ................
80400060: 00000000 00000000 00000000 00000000    ................
80400070: 00000000 00000000 00000000 00000000    ................
80400080: 00000000 00000000 00000000 00000000    ................
80400090: 00000000 00000000 00000000 00000000    ................
804000a0: 00000000 00000000 00000000 00000000    ................
804000b0: 00000000 00000000 00000000 00000000    ................
804000c0: 00000000 00000000 00000000 00000000    ................
804000d0: 00000000 00000000 00000000 00000000    ................
804000e0: 00000000 00000000 00000000 00000000    ................
804000f0: 00000000 00000000 00000000 00000000    ................

But it can read the serial number at offset 0x3f0

=> misc read  otp@10070000 3f0 80400010 10
=> md 80400000
80400000: ffffffff ffffffff ffffffff ffffffff    ................
80400010: 00000001 fffffffe ffffffff ffffffff    ................
80400020: 00000000 00000000 00000000 00000000    ................
80400030: 00000000 00000000 00000000 00000000    ................
80400040: 00000000 00000000 00000000 00000000    ................
80400050: 00000000 00000000 00000000 00000000    ................
80400060: 00000000 00000000 00000000 00000000    ................
80400070: 00000000 00000000 00000000 00000000    ................
80400080: 00000000 00000000 00000000 00000000    ................
80400090: 00000000 00000000 00000000 00000000    ................
804000a0: 00000000 00000000 00000000 00000000    ................
804000b0: 00000000 00000000 00000000 00000000    ................
804000c0: 00000000 00000000 00000000 00000000    ................
804000d0: 00000000 00000000 00000000 00000000    ................
804000e0: 00000000 00000000 00000000 00000000    ................
804000f0: 00000000 00000000 00000000 00000000    ................

Regards,
Bin

Re: [RFC PATCH v6 2/2] hw/misc/sifive_u_otp: Add backend drive support

[Bin Meng]

On Mon, Sep 28, 2020 at 6:12 PM Green Wan <green.wan@sifive.com> wrote:
>
> 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

nits: please prefix the command with a leading "$ ", like

$ 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>
> ---
>  hw/misc/sifive_u_otp.c         | 51 ++++++++++++++++++++++++++++++++++
>  include/hw/misc/sifive_u_otp.h |  2 ++
>  2 files changed, 53 insertions(+)
>
> diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
> index 685c1f8e07..f2585c1ed7 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, &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 +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)
> 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;
>  };
>

Please add the serial number initialization for the block backend as
well in sifive_u_otp_reset().

The logic should be something like testing offset 0x3f0 and 0x3f4
numbers to see if they follow the rule: value 0x3f0 == ~(value 0x3f4).
If not, the data should be initialized per the `serial` property from
the QEMU object.

Regards,
Bin

Re: [RFC PATCH v6 1/2] hw/misc/sifive_u_otp: Add write function and write-once protection

[Green Wan]

On Wed, Oct 14, 2020 at 1:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Green,
>
> On Mon, Sep 28, 2020 at 6:12 PM Green Wan <green.wan@sifive.com> wrote:
> >
> >  - Add write operation to update fuse data bit when PWE bit is on.
> >  - Add array, fuse_wo, to store the 'written' status for all bits
> >    of OTP to block the write operation.
> >
> > Signed-off-by: Green Wan <green.wan@sifive.com>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/misc/sifive_u_otp.c         | 30 +++++++++++++++++++++++++++++-
> >  include/hw/misc/sifive_u_otp.h |  3 +++
> >  2 files changed, 32 insertions(+), 1 deletion(-)
> >
>
> I am not sure how you tested this. I wrote a simple U-Boot command to
> call U-Boot sifive-otp driver to test the write functionality, but it
> failed.
>
> => misc write otp@10070000 0 80200000 10
                                                      ^^^^^^^^^
Quick ask, how about 'md 80200000'?

I didn't use 'misc write' command. I can check afterward.

> => misc read  otp@10070000 0 80400000 10
> => md 80400000
> 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> 80400010: 00000000 00000000 00000000 00000000    ................
> 80400020: 00000000 00000000 00000000 00000000    ................
> 80400030: 00000000 00000000 00000000 00000000    ................
> 80400040: 00000000 00000000 00000000 00000000    ................
> 80400050: 00000000 00000000 00000000 00000000    ................
> 80400060: 00000000 00000000 00000000 00000000    ................
> 80400070: 00000000 00000000 00000000 00000000    ................
> 80400080: 00000000 00000000 00000000 00000000    ................
> 80400090: 00000000 00000000 00000000 00000000    ................
> 804000a0: 00000000 00000000 00000000 00000000    ................
> 804000b0: 00000000 00000000 00000000 00000000    ................
> 804000c0: 00000000 00000000 00000000 00000000    ................
> 804000d0: 00000000 00000000 00000000 00000000    ................
> 804000e0: 00000000 00000000 00000000 00000000    ................
> 804000f0: 00000000 00000000 00000000 00000000    ................
> => misc write otp@10070000 0 80200010 10
> => misc read  otp@10070000 0 80400010 10
> => md 80400000
> 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> 80400010: ffffffff ffffffff ffffffff ffffffff    ................
> 80400020: 00000000 00000000 00000000 00000000    ................
> 80400030: 00000000 00000000 00000000 00000000    ................
> 80400040: 00000000 00000000 00000000 00000000    ................
> 80400050: 00000000 00000000 00000000 00000000    ................
> 80400060: 00000000 00000000 00000000 00000000    ................
> 80400070: 00000000 00000000 00000000 00000000    ................
> 80400080: 00000000 00000000 00000000 00000000    ................
> 80400090: 00000000 00000000 00000000 00000000    ................
> 804000a0: 00000000 00000000 00000000 00000000    ................
> 804000b0: 00000000 00000000 00000000 00000000    ................
> 804000c0: 00000000 00000000 00000000 00000000    ................
> 804000d0: 00000000 00000000 00000000 00000000    ................
> 804000e0: 00000000 00000000 00000000 00000000    ................
> 804000f0: 00000000 00000000 00000000 00000000    ................
>
> But it can read the serial number at offset 0x3f0
>
> => misc read  otp@10070000 3f0 80400010 10
> => md 80400000
> 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> 80400010: 00000001 fffffffe ffffffff ffffffff    ................
> 80400020: 00000000 00000000 00000000 00000000    ................
> 80400030: 00000000 00000000 00000000 00000000    ................
> 80400040: 00000000 00000000 00000000 00000000    ................
> 80400050: 00000000 00000000 00000000 00000000    ................
> 80400060: 00000000 00000000 00000000 00000000    ................
> 80400070: 00000000 00000000 00000000 00000000    ................
> 80400080: 00000000 00000000 00000000 00000000    ................
> 80400090: 00000000 00000000 00000000 00000000    ................
> 804000a0: 00000000 00000000 00000000 00000000    ................
> 804000b0: 00000000 00000000 00000000 00000000    ................
> 804000c0: 00000000 00000000 00000000 00000000    ................
> 804000d0: 00000000 00000000 00000000 00000000    ................
> 804000e0: 00000000 00000000 00000000 00000000    ................
> 804000f0: 00000000 00000000 00000000 00000000    ................
>
> Regards,
> Bin

Re: [RFC PATCH v6 1/2] hw/misc/sifive_u_otp: Add write function and write-once protection

[Bin Meng]

Hi Green,

On Wed, Oct 14, 2020 at 3:02 PM Green Wan <green.wan@sifive.com> wrote:
>
> On Wed, Oct 14, 2020 at 1:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Green,
> >
> > On Mon, Sep 28, 2020 at 6:12 PM Green Wan <green.wan@sifive.com> wrote:
> > >
> > >  - Add write operation to update fuse data bit when PWE bit is on.
> > >  - Add array, fuse_wo, to store the 'written' status for all bits
> > >    of OTP to block the write operation.
> > >
> > > Signed-off-by: Green Wan <green.wan@sifive.com>
> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > ---
> > >  hw/misc/sifive_u_otp.c         | 30 +++++++++++++++++++++++++++++-
> > >  include/hw/misc/sifive_u_otp.h |  3 +++
> > >  2 files changed, 32 insertions(+), 1 deletion(-)
> > >
> >
> > I am not sure how you tested this. I wrote a simple U-Boot command to
> > call U-Boot sifive-otp driver to test the write functionality, but it
> > failed.
> >
> > => misc write otp@10070000 0 80200000 10
>                                                       ^^^^^^^^^
> Quick ask, how about 'md 80200000'?
>
> I didn't use 'misc write' command. I can check afterward.

Note 'misc write' is a new U-Boot command I just added for testing
this QEMU functionality. Please use the U-Boot patch below:
http://patchwork.ozlabs.org/project/uboot/patch/1602657292-82815-1-git-send-email-bmeng.cn@gmail.com/

>
> > => misc read  otp@10070000 0 80400000 10
> > => md 80400000
> > 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> > 80400010: 00000000 00000000 00000000 00000000    ................
> > 80400020: 00000000 00000000 00000000 00000000    ................
> > 80400030: 00000000 00000000 00000000 00000000    ................
> > 80400040: 00000000 00000000 00000000 00000000    ................
> > 80400050: 00000000 00000000 00000000 00000000    ................
> > 80400060: 00000000 00000000 00000000 00000000    ................
> > 80400070: 00000000 00000000 00000000 00000000    ................
> > 80400080: 00000000 00000000 00000000 00000000    ................
> > 80400090: 00000000 00000000 00000000 00000000    ................
> > 804000a0: 00000000 00000000 00000000 00000000    ................
> > 804000b0: 00000000 00000000 00000000 00000000    ................
> > 804000c0: 00000000 00000000 00000000 00000000    ................
> > 804000d0: 00000000 00000000 00000000 00000000    ................
> > 804000e0: 00000000 00000000 00000000 00000000    ................
> > 804000f0: 00000000 00000000 00000000 00000000    ................
> > => misc write otp@10070000 0 80200010 10
> > => misc read  otp@10070000 0 80400010 10
> > => md 80400000
> > 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> > 80400010: ffffffff ffffffff ffffffff ffffffff    ................
> > 80400020: 00000000 00000000 00000000 00000000    ................
> > 80400030: 00000000 00000000 00000000 00000000    ................
> > 80400040: 00000000 00000000 00000000 00000000    ................
> > 80400050: 00000000 00000000 00000000 00000000    ................
> > 80400060: 00000000 00000000 00000000 00000000    ................
> > 80400070: 00000000 00000000 00000000 00000000    ................
> > 80400080: 00000000 00000000 00000000 00000000    ................
> > 80400090: 00000000 00000000 00000000 00000000    ................
> > 804000a0: 00000000 00000000 00000000 00000000    ................
> > 804000b0: 00000000 00000000 00000000 00000000    ................
> > 804000c0: 00000000 00000000 00000000 00000000    ................
> > 804000d0: 00000000 00000000 00000000 00000000    ................
> > 804000e0: 00000000 00000000 00000000 00000000    ................
> > 804000f0: 00000000 00000000 00000000 00000000    ................
> >
> > But it can read the serial number at offset 0x3f0
> >
> > => misc read  otp@10070000 3f0 80400010 10
> > => md 80400000
> > 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> > 80400010: 00000001 fffffffe ffffffff ffffffff    ................
> > 80400020: 00000000 00000000 00000000 00000000    ................
> > 80400030: 00000000 00000000 00000000 00000000    ................
> > 80400040: 00000000 00000000 00000000 00000000    ................
> > 80400050: 00000000 00000000 00000000 00000000    ................
> > 80400060: 00000000 00000000 00000000 00000000    ................
> > 80400070: 00000000 00000000 00000000 00000000    ................
> > 80400080: 00000000 00000000 00000000 00000000    ................
> > 80400090: 00000000 00000000 00000000 00000000    ................
> > 804000a0: 00000000 00000000 00000000 00000000    ................
> > 804000b0: 00000000 00000000 00000000 00000000    ................
> > 804000c0: 00000000 00000000 00000000 00000000    ................
> > 804000d0: 00000000 00000000 00000000 00000000    ................
> > 804000e0: 00000000 00000000 00000000 00000000    ................
> > 804000f0: 00000000 00000000 00000000 00000000    ................

Regards,
Bin

Re: [RFC PATCH v6 1/2] hw/misc/sifive_u_otp: Add write function and write-once protection

[Green Wan]

On Wed, Oct 14, 2020 at 3:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Green,
>
> On Wed, Oct 14, 2020 at 3:02 PM Green Wan <green.wan@sifive.com> wrote:
> >
> > On Wed, Oct 14, 2020 at 1:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Green,
> > >
> > > On Mon, Sep 28, 2020 at 6:12 PM Green Wan <green.wan@sifive.com> wrote:
> > > >
> > > >  - Add write operation to update fuse data bit when PWE bit is on.
> > > >  - Add array, fuse_wo, to store the 'written' status for all bits
> > > >    of OTP to block the write operation.
> > > >
> > > > Signed-off-by: Green Wan <green.wan@sifive.com>
> > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > > ---
> > > >  hw/misc/sifive_u_otp.c         | 30 +++++++++++++++++++++++++++++-
> > > >  include/hw/misc/sifive_u_otp.h |  3 +++
> > > >  2 files changed, 32 insertions(+), 1 deletion(-)
> > > >
> > >
> > > I am not sure how you tested this. I wrote a simple U-Boot command to
> > > call U-Boot sifive-otp driver to test the write functionality, but it
> > > failed.
> > >
> > > => misc write otp@10070000 0 80200000 10
> >                                                       ^^^^^^^^^
> > Quick ask, how about 'md 80200000'?
> >
> > I didn't use 'misc write' command. I can check afterward.
>
> Note 'misc write' is a new U-Boot command I just added for testing
> this QEMU functionality. Please use the U-Boot patch below:
> http://patchwork.ozlabs.org/project/uboot/patch/1602657292-82815-1-git-send-email-bmeng.cn@gmail.com/
>
Thanks for pointing it out.

I've found one bug when I revise the macro of the write function and
the read is correct. It's my mistake. I will include and rerun this
test as well.

> >
> > > => misc read  otp@10070000 0 80400000 10
> > > => md 80400000
> > > 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> > > 80400010: 00000000 00000000 00000000 00000000    ................
> > > 80400020: 00000000 00000000 00000000 00000000    ................
> > > 80400030: 00000000 00000000 00000000 00000000    ................
> > > 80400040: 00000000 00000000 00000000 00000000    ................
> > > 80400050: 00000000 00000000 00000000 00000000    ................
> > > 80400060: 00000000 00000000 00000000 00000000    ................
> > > 80400070: 00000000 00000000 00000000 00000000    ................
> > > 80400080: 00000000 00000000 00000000 00000000    ................
> > > 80400090: 00000000 00000000 00000000 00000000    ................
> > > 804000a0: 00000000 00000000 00000000 00000000    ................
> > > 804000b0: 00000000 00000000 00000000 00000000    ................
> > > 804000c0: 00000000 00000000 00000000 00000000    ................
> > > 804000d0: 00000000 00000000 00000000 00000000    ................
> > > 804000e0: 00000000 00000000 00000000 00000000    ................
> > > 804000f0: 00000000 00000000 00000000 00000000    ................
> > > => misc write otp@10070000 0 80200010 10
> > > => misc read  otp@10070000 0 80400010 10
> > > => md 80400000
> > > 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> > > 80400010: ffffffff ffffffff ffffffff ffffffff    ................
> > > 80400020: 00000000 00000000 00000000 00000000    ................
> > > 80400030: 00000000 00000000 00000000 00000000    ................
> > > 80400040: 00000000 00000000 00000000 00000000    ................
> > > 80400050: 00000000 00000000 00000000 00000000    ................
> > > 80400060: 00000000 00000000 00000000 00000000    ................
> > > 80400070: 00000000 00000000 00000000 00000000    ................
> > > 80400080: 00000000 00000000 00000000 00000000    ................
> > > 80400090: 00000000 00000000 00000000 00000000    ................
> > > 804000a0: 00000000 00000000 00000000 00000000    ................
> > > 804000b0: 00000000 00000000 00000000 00000000    ................
> > > 804000c0: 00000000 00000000 00000000 00000000    ................
> > > 804000d0: 00000000 00000000 00000000 00000000    ................
> > > 804000e0: 00000000 00000000 00000000 00000000    ................
> > > 804000f0: 00000000 00000000 00000000 00000000    ................
> > >
> > > But it can read the serial number at offset 0x3f0
> > >
> > > => misc read  otp@10070000 3f0 80400010 10
> > > => md 80400000
> > > 80400000: ffffffff ffffffff ffffffff ffffffff    ................
> > > 80400010: 00000001 fffffffe ffffffff ffffffff    ................
> > > 80400020: 00000000 00000000 00000000 00000000    ................
> > > 80400030: 00000000 00000000 00000000 00000000    ................
> > > 80400040: 00000000 00000000 00000000 00000000    ................
> > > 80400050: 00000000 00000000 00000000 00000000    ................
> > > 80400060: 00000000 00000000 00000000 00000000    ................
> > > 80400070: 00000000 00000000 00000000 00000000    ................
> > > 80400080: 00000000 00000000 00000000 00000000    ................
> > > 80400090: 00000000 00000000 00000000 00000000    ................
> > > 804000a0: 00000000 00000000 00000000 00000000    ................
> > > 804000b0: 00000000 00000000 00000000 00000000    ................
> > > 804000c0: 00000000 00000000 00000000 00000000    ................
> > > 804000d0: 00000000 00000000 00000000 00000000    ................
> > > 804000e0: 00000000 00000000 00000000 00000000    ................
> > > 804000f0: 00000000 00000000 00000000 00000000    ................
>
> Regards,
> Bin