All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] Add write-once and file-backed features to OTP
@ 2020-07-24  9:51 ` Green Wan
  0 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-24  9:51 UTC (permalink / raw)
  Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, qemu-devel,
	Green Wan, Alistair Francis, Paolo Bonzini, Palmer Dabbelt,
	bmeng.cn

Add file-backed implementation to allow users to assign an OTP image
file to machine. If '-boot otp-file=filename' is specified, OTP 
device uses otp image file instead of fuse array. In order to keep data
up-to-date due to unexpected crash or CTRL+a-x, every read/write to OTP
memory involves file open, mmap and close operation to the image file.

Add write-once feature to block second write operation to the OTP
memory. Only keep the 'written' state for non-control register range
from 0x38 to 16KB.

Tested on sifive_u for both qemu and u-boot.

Green Wan (2):
  hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to
    boot option
  hw/riscv: sifive_u: Add write-once protection.

 hw/riscv/sifive_u_otp.c         | 122 ++++++++++++++++++++++++++++++--
 include/hw/riscv/sifive_u_otp.h |   3 +
 qemu-options.hx                 |   3 +-
 softmmu/vl.c                    |   6 +-
 4 files changed, 128 insertions(+), 6 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH 0/2] Add write-once and file-backed features to OTP
@ 2020-07-24  9:51 ` Green Wan
  0 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-24  9:51 UTC (permalink / raw)
  Cc: bmeng.cn, Green Wan, Palmer Dabbelt, Alistair Francis,
	Sagar Karandikar, Bastian Koppelmann, Paolo Bonzini, qemu-riscv,
	qemu-devel

Add file-backed implementation to allow users to assign an OTP image
file to machine. If '-boot otp-file=filename' is specified, OTP 
device uses otp image file instead of fuse array. In order to keep data
up-to-date due to unexpected crash or CTRL+a-x, every read/write to OTP
memory involves file open, mmap and close operation to the image file.

Add write-once feature to block second write operation to the OTP
memory. Only keep the 'written' state for non-control register range
from 0x38 to 16KB.

Tested on sifive_u for both qemu and u-boot.

Green Wan (2):
  hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to
    boot option
  hw/riscv: sifive_u: Add write-once protection.

 hw/riscv/sifive_u_otp.c         | 122 ++++++++++++++++++++++++++++++--
 include/hw/riscv/sifive_u_otp.h |   3 +
 qemu-options.hx                 |   3 +-
 softmmu/vl.c                    |   6 +-
 4 files changed, 128 insertions(+), 6 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
  2020-07-24  9:51 ` Green Wan
@ 2020-07-24  9:51   ` Green Wan
  -1 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-24  9:51 UTC (permalink / raw)
  Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, qemu-devel,
	Green Wan, Alistair Francis, Paolo Bonzini, Palmer Dabbelt,
	bmeng.cn

Add a file-backed implementation for OTP of sifive_u machine. Use
'-boot otp-file=xxx' to enable it. Do file open, mmap and close
for every OTP read/write in case keep the update-to-date snapshot
of OTP.

Signed-off-by: Green Wan <green.wan@sifive.com>
---
 hw/riscv/sifive_u_otp.c         | 88 ++++++++++++++++++++++++++++++++-
 include/hw/riscv/sifive_u_otp.h |  2 +
 qemu-options.hx                 |  3 +-
 softmmu/vl.c                    |  6 ++-
 4 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
index f6ecbaa2ca..26e1965821 100644
--- a/hw/riscv/sifive_u_otp.c
+++ b/hw/riscv/sifive_u_otp.c
@@ -24,6 +24,72 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "hw/riscv/sifive_u_otp.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <string.h>
+
+#define TRACE_PREFIX            "FU540_OTP: "
+#define SIFIVE_FU540_OTP_SIZE   (SIFIVE_U_OTP_NUM_FUSES * 4)
+
+static int otp_backed_fd;
+static unsigned int *otp_mmap;
+
+static void sifive_u_otp_backed_load(const char *filename);
+static uint64_t sifive_u_otp_backed_read(uint32_t fuseidx);
+static void sifive_u_otp_backed_write(uint32_t fuseidx,
+                                      uint32_t paio,
+                                      uint32_t pdin);
+static void sifive_u_otp_backed_unload(void);
+
+void sifive_u_otp_backed_load(const char *filename)
+{
+    if (otp_backed_fd < 0) {
+
+        otp_backed_fd = open(filename, O_RDWR);
+
+        if (otp_backed_fd < 0)
+            qemu_log_mask(LOG_TRACE,
+                          TRACE_PREFIX "Warning: can't open otp file\n");
+        else {
+
+            otp_mmap = (unsigned int *)mmap(0,
+                                            SIFIVE_FU540_OTP_SIZE,
+                                            PROT_READ | PROT_WRITE | PROT_EXEC,
+                                            MAP_FILE | MAP_SHARED,
+                                            otp_backed_fd,
+                                            0);
+
+            if (otp_mmap == MAP_FAILED)
+                qemu_log_mask(LOG_TRACE,
+                              TRACE_PREFIX "Warning: can't mmap otp file\n");
+        }
+    }
+
+}
+
+uint64_t sifive_u_otp_backed_read(uint32_t fuseidx)
+{
+    return (uint64_t)(otp_mmap[fuseidx]);
+}
+
+void sifive_u_otp_backed_write(uint32_t fuseidx, uint32_t paio, uint32_t pdin)
+{
+    otp_mmap[fuseidx] &= ~(pdin << paio);
+    otp_mmap[fuseidx] |= (pdin << paio);
+}
+
+
+void sifive_u_otp_backed_unload(void)
+{
+    munmap(otp_mmap, SIFIVE_FU540_OTP_SIZE);
+    close(otp_backed_fd);
+    otp_backed_fd = -1;
+}
 
 static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -46,7 +112,17 @@ static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
         if ((s->pce & SIFIVE_U_OTP_PCE_EN) &&
             (s->pdstb & SIFIVE_U_OTP_PDSTB_EN) &&
             (s->ptrim & SIFIVE_U_OTP_PTRIM_EN)) {
-            return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
+
+            if (otp_file) {
+                uint64_t val;
+
+                sifive_u_otp_backed_load(otp_file);
+                val = sifive_u_otp_backed_read(s->pa);
+                sifive_u_otp_backed_unload();
+
+                return val;
+            } else
+                return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
         } else {
             return 0xff;
         }
@@ -123,6 +199,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
         s->ptrim = val32;
         break;
     case SIFIVE_U_OTP_PWE:
+        if (otp_file) {
+            sifive_u_otp_backed_load(otp_file);
+            sifive_u_otp_backed_write(s->pa, s->paio, s->pdin);
+            sifive_u_otp_backed_unload();
+        }
+
         s->pwe = val32;
         break;
     default:
@@ -165,6 +247,10 @@ static void sifive_u_otp_reset(DeviceState *dev)
     /* Make a valid content of serial number */
     s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
     s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
+
+    /* Initialize file mmap and descriptor. */
+    otp_mmap = NULL;
+    otp_backed_fd = -1;
 }
 
 static void sifive_u_otp_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
index 639297564a..1342bd7342 100644
--- a/include/hw/riscv/sifive_u_otp.h
+++ b/include/hw/riscv/sifive_u_otp.h
@@ -52,6 +52,8 @@
 #define SIFIVE_U_OTP(obj) \
     OBJECT_CHECK(SiFiveUOTPState, (obj), TYPE_SIFIVE_U_OTP)
 
+extern const char *otp_file;
+
 typedef struct SiFiveUOTPState {
     /*< private >*/
     SysBusDevice parent_obj;
diff --git a/qemu-options.hx b/qemu-options.hx
index 708583b4ce..eb9a54f4ed 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -415,10 +415,11 @@ ERST
 
 DEF("boot", HAS_ARG, QEMU_OPTION_boot,
     "-boot [order=drives][,once=drives][,menu=on|off]\n"
-    "      [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n"
+    "      [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off][,otp-file=otp_file]\n"
     "                'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)\n"
     "                'sp_name': the file's name that would be passed to bios as logo picture, if menu=on\n"
     "                'sp_time': the period that splash picture last if menu=on, unit is ms\n"
+    "                'otp_file': the file name backed OTP\n"
     "                'rb_timeout': the timeout before guest reboot when boot failed, unit is ms\n",
     QEMU_ARCH_ALL)
 SRST
diff --git a/softmmu/vl.c b/softmmu/vl.c
index f476ef89ed..58e0b2fc0a 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -21,7 +21,6 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/units.h"
@@ -161,6 +160,7 @@ unsigned int nb_prom_envs = 0;
 const char *prom_envs[MAX_PROM_ENVS];
 int boot_menu;
 bool boot_strict;
+const char *otp_file;
 uint8_t *boot_splash_filedata;
 int only_migratable; /* turn it off unless user states otherwise */
 bool wakeup_suspend_enabled;
@@ -308,6 +308,9 @@ static QemuOptsList qemu_boot_opts = {
         }, {
             .name = "strict",
             .type = QEMU_OPT_BOOL,
+        }, {
+            .name = "otp-file",
+            .type = QEMU_OPT_STRING,
         },
         { /*End of list */ }
     },
@@ -4215,6 +4218,7 @@ void qemu_init(int argc, char **argv, char **envp)
 
         boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu);
         boot_strict = qemu_opt_get_bool(opts, "strict", false);
+        otp_file = qemu_opt_get(opts, "otp-file");
     }
 
     if (!boot_order) {
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
@ 2020-07-24  9:51   ` Green Wan
  0 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-24  9:51 UTC (permalink / raw)
  Cc: bmeng.cn, Green Wan, Palmer Dabbelt, Alistair Francis,
	Sagar Karandikar, Bastian Koppelmann, Paolo Bonzini, qemu-riscv,
	qemu-devel

Add a file-backed implementation for OTP of sifive_u machine. Use
'-boot otp-file=xxx' to enable it. Do file open, mmap and close
for every OTP read/write in case keep the update-to-date snapshot
of OTP.

Signed-off-by: Green Wan <green.wan@sifive.com>
---
 hw/riscv/sifive_u_otp.c         | 88 ++++++++++++++++++++++++++++++++-
 include/hw/riscv/sifive_u_otp.h |  2 +
 qemu-options.hx                 |  3 +-
 softmmu/vl.c                    |  6 ++-
 4 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
index f6ecbaa2ca..26e1965821 100644
--- a/hw/riscv/sifive_u_otp.c
+++ b/hw/riscv/sifive_u_otp.c
@@ -24,6 +24,72 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "hw/riscv/sifive_u_otp.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <string.h>
+
+#define TRACE_PREFIX            "FU540_OTP: "
+#define SIFIVE_FU540_OTP_SIZE   (SIFIVE_U_OTP_NUM_FUSES * 4)
+
+static int otp_backed_fd;
+static unsigned int *otp_mmap;
+
+static void sifive_u_otp_backed_load(const char *filename);
+static uint64_t sifive_u_otp_backed_read(uint32_t fuseidx);
+static void sifive_u_otp_backed_write(uint32_t fuseidx,
+                                      uint32_t paio,
+                                      uint32_t pdin);
+static void sifive_u_otp_backed_unload(void);
+
+void sifive_u_otp_backed_load(const char *filename)
+{
+    if (otp_backed_fd < 0) {
+
+        otp_backed_fd = open(filename, O_RDWR);
+
+        if (otp_backed_fd < 0)
+            qemu_log_mask(LOG_TRACE,
+                          TRACE_PREFIX "Warning: can't open otp file\n");
+        else {
+
+            otp_mmap = (unsigned int *)mmap(0,
+                                            SIFIVE_FU540_OTP_SIZE,
+                                            PROT_READ | PROT_WRITE | PROT_EXEC,
+                                            MAP_FILE | MAP_SHARED,
+                                            otp_backed_fd,
+                                            0);
+
+            if (otp_mmap == MAP_FAILED)
+                qemu_log_mask(LOG_TRACE,
+                              TRACE_PREFIX "Warning: can't mmap otp file\n");
+        }
+    }
+
+}
+
+uint64_t sifive_u_otp_backed_read(uint32_t fuseidx)
+{
+    return (uint64_t)(otp_mmap[fuseidx]);
+}
+
+void sifive_u_otp_backed_write(uint32_t fuseidx, uint32_t paio, uint32_t pdin)
+{
+    otp_mmap[fuseidx] &= ~(pdin << paio);
+    otp_mmap[fuseidx] |= (pdin << paio);
+}
+
+
+void sifive_u_otp_backed_unload(void)
+{
+    munmap(otp_mmap, SIFIVE_FU540_OTP_SIZE);
+    close(otp_backed_fd);
+    otp_backed_fd = -1;
+}
 
 static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -46,7 +112,17 @@ static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
         if ((s->pce & SIFIVE_U_OTP_PCE_EN) &&
             (s->pdstb & SIFIVE_U_OTP_PDSTB_EN) &&
             (s->ptrim & SIFIVE_U_OTP_PTRIM_EN)) {
-            return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
+
+            if (otp_file) {
+                uint64_t val;
+
+                sifive_u_otp_backed_load(otp_file);
+                val = sifive_u_otp_backed_read(s->pa);
+                sifive_u_otp_backed_unload();
+
+                return val;
+            } else
+                return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
         } else {
             return 0xff;
         }
@@ -123,6 +199,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
         s->ptrim = val32;
         break;
     case SIFIVE_U_OTP_PWE:
+        if (otp_file) {
+            sifive_u_otp_backed_load(otp_file);
+            sifive_u_otp_backed_write(s->pa, s->paio, s->pdin);
+            sifive_u_otp_backed_unload();
+        }
+
         s->pwe = val32;
         break;
     default:
@@ -165,6 +247,10 @@ static void sifive_u_otp_reset(DeviceState *dev)
     /* Make a valid content of serial number */
     s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
     s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
+
+    /* Initialize file mmap and descriptor. */
+    otp_mmap = NULL;
+    otp_backed_fd = -1;
 }
 
 static void sifive_u_otp_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
index 639297564a..1342bd7342 100644
--- a/include/hw/riscv/sifive_u_otp.h
+++ b/include/hw/riscv/sifive_u_otp.h
@@ -52,6 +52,8 @@
 #define SIFIVE_U_OTP(obj) \
     OBJECT_CHECK(SiFiveUOTPState, (obj), TYPE_SIFIVE_U_OTP)
 
+extern const char *otp_file;
+
 typedef struct SiFiveUOTPState {
     /*< private >*/
     SysBusDevice parent_obj;
diff --git a/qemu-options.hx b/qemu-options.hx
index 708583b4ce..eb9a54f4ed 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -415,10 +415,11 @@ ERST
 
 DEF("boot", HAS_ARG, QEMU_OPTION_boot,
     "-boot [order=drives][,once=drives][,menu=on|off]\n"
-    "      [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n"
+    "      [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off][,otp-file=otp_file]\n"
     "                'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)\n"
     "                'sp_name': the file's name that would be passed to bios as logo picture, if menu=on\n"
     "                'sp_time': the period that splash picture last if menu=on, unit is ms\n"
+    "                'otp_file': the file name backed OTP\n"
     "                'rb_timeout': the timeout before guest reboot when boot failed, unit is ms\n",
     QEMU_ARCH_ALL)
 SRST
diff --git a/softmmu/vl.c b/softmmu/vl.c
index f476ef89ed..58e0b2fc0a 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -21,7 +21,6 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/units.h"
@@ -161,6 +160,7 @@ unsigned int nb_prom_envs = 0;
 const char *prom_envs[MAX_PROM_ENVS];
 int boot_menu;
 bool boot_strict;
+const char *otp_file;
 uint8_t *boot_splash_filedata;
 int only_migratable; /* turn it off unless user states otherwise */
 bool wakeup_suspend_enabled;
@@ -308,6 +308,9 @@ static QemuOptsList qemu_boot_opts = {
         }, {
             .name = "strict",
             .type = QEMU_OPT_BOOL,
+        }, {
+            .name = "otp-file",
+            .type = QEMU_OPT_STRING,
         },
         { /*End of list */ }
     },
@@ -4215,6 +4218,7 @@ void qemu_init(int argc, char **argv, char **envp)
 
         boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu);
         boot_strict = qemu_opt_get_bool(opts, "strict", false);
+        otp_file = qemu_opt_get(opts, "otp-file");
     }
 
     if (!boot_order) {
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH 2/2] hw/riscv: sifive_u: Add write-once protection.
  2020-07-24  9:51 ` Green Wan
@ 2020-07-24  9:51   ` Green Wan
  -1 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-24  9:51 UTC (permalink / raw)
  Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, qemu-devel,
	Green Wan, Alistair Francis, Paolo Bonzini, Palmer Dabbelt,
	bmeng.cn

Add array to store the 'written' status for all bit of OTP to block
the write operation to the same bit. Ignore the control register
offset from 0x0 to 0x38 of OTP memory mapping.

Signed-off-by: Green Wan <green.wan@sifive.com>
---
 hw/riscv/sifive_u_otp.c         | 34 ++++++++++++++++++++++++++++++---
 include/hw/riscv/sifive_u_otp.h |  1 +
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
index 26e1965821..e0f85dee22 100644
--- a/hw/riscv/sifive_u_otp.c
+++ b/hw/riscv/sifive_u_otp.c
@@ -36,6 +36,12 @@
 #define TRACE_PREFIX            "FU540_OTP: "
 #define SIFIVE_FU540_OTP_SIZE   (SIFIVE_U_OTP_NUM_FUSES * 4)
 
+#define SET_WRITTEN_BIT(map, idx, bit)    \
+    (map[idx] |= (0x1 << bit))
+
+#define GET_WRITTEN_BIT(map, idx, bit)    \
+    ((map[idx] >> bit) & 0x1)
+
 static int otp_backed_fd;
 static unsigned int *otp_mmap;
 
@@ -199,6 +205,18 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
         s->ptrim = val32;
         break;
     case SIFIVE_U_OTP_PWE:
+        /* Keep written state for data only and PWE is enabled. Ignore PAS=1 */
+        if ((s->pa > SIFIVE_U_OTP_PWE) && (val32 & 0x1) && !s->pas) {
+            if (GET_WRITTEN_BIT(s->fuse_wo, s->pa, s->paio)) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              TRACE_PREFIX "Error: write idx<%u>, bit<%u>\n",
+                              s->pa, s->paio);
+                break;
+            } else {
+                SET_WRITTEN_BIT(s->fuse_wo, s->pa, s->paio);
+            }
+        }
+
         if (otp_file) {
             sifive_u_otp_backed_load(otp_file);
             sifive_u_otp_backed_write(s->pa, s->paio, s->pdin);
@@ -244,9 +262,19 @@ static void sifive_u_otp_reset(DeviceState *dev)
     /* Initialize all fuses' initial value to 0xFFs */
     memset(s->fuse, 0xff, sizeof(s->fuse));
 
-    /* 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));
+
+    /* if otp file is used, not over write these value. */
+    if (!otp_file) {
+        /* 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);
+
+        /* set status to 'written' */
+        s->fuse_wo[SIFIVE_U_OTP_SERIAL_ADDR] = 0xffff;
+        s->fuse_wo[SIFIVE_U_OTP_SERIAL_ADDR + 1] = 0xffff;
+    }
 
     /* Initialize file mmap and descriptor. */
     otp_mmap = NULL;
diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
index 1342bd7342..9c9c57f39e 100644
--- a/include/hw/riscv/sifive_u_otp.h
+++ b/include/hw/riscv/sifive_u_otp.h
@@ -77,6 +77,7 @@ typedef struct SiFiveUOTPState {
     uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
     /* config */
     uint32_t serial;
+    uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
 } SiFiveUOTPState;
 
 #endif /* HW_SIFIVE_U_OTP_H */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH 2/2] hw/riscv: sifive_u: Add write-once protection.
@ 2020-07-24  9:51   ` Green Wan
  0 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-24  9:51 UTC (permalink / raw)
  Cc: bmeng.cn, Green Wan, Palmer Dabbelt, Alistair Francis,
	Sagar Karandikar, Bastian Koppelmann, Paolo Bonzini, qemu-riscv,
	qemu-devel

Add array to store the 'written' status for all bit of OTP to block
the write operation to the same bit. Ignore the control register
offset from 0x0 to 0x38 of OTP memory mapping.

Signed-off-by: Green Wan <green.wan@sifive.com>
---
 hw/riscv/sifive_u_otp.c         | 34 ++++++++++++++++++++++++++++++---
 include/hw/riscv/sifive_u_otp.h |  1 +
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
index 26e1965821..e0f85dee22 100644
--- a/hw/riscv/sifive_u_otp.c
+++ b/hw/riscv/sifive_u_otp.c
@@ -36,6 +36,12 @@
 #define TRACE_PREFIX            "FU540_OTP: "
 #define SIFIVE_FU540_OTP_SIZE   (SIFIVE_U_OTP_NUM_FUSES * 4)
 
+#define SET_WRITTEN_BIT(map, idx, bit)    \
+    (map[idx] |= (0x1 << bit))
+
+#define GET_WRITTEN_BIT(map, idx, bit)    \
+    ((map[idx] >> bit) & 0x1)
+
 static int otp_backed_fd;
 static unsigned int *otp_mmap;
 
@@ -199,6 +205,18 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
         s->ptrim = val32;
         break;
     case SIFIVE_U_OTP_PWE:
+        /* Keep written state for data only and PWE is enabled. Ignore PAS=1 */
+        if ((s->pa > SIFIVE_U_OTP_PWE) && (val32 & 0x1) && !s->pas) {
+            if (GET_WRITTEN_BIT(s->fuse_wo, s->pa, s->paio)) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              TRACE_PREFIX "Error: write idx<%u>, bit<%u>\n",
+                              s->pa, s->paio);
+                break;
+            } else {
+                SET_WRITTEN_BIT(s->fuse_wo, s->pa, s->paio);
+            }
+        }
+
         if (otp_file) {
             sifive_u_otp_backed_load(otp_file);
             sifive_u_otp_backed_write(s->pa, s->paio, s->pdin);
@@ -244,9 +262,19 @@ static void sifive_u_otp_reset(DeviceState *dev)
     /* Initialize all fuses' initial value to 0xFFs */
     memset(s->fuse, 0xff, sizeof(s->fuse));
 
-    /* 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));
+
+    /* if otp file is used, not over write these value. */
+    if (!otp_file) {
+        /* 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);
+
+        /* set status to 'written' */
+        s->fuse_wo[SIFIVE_U_OTP_SERIAL_ADDR] = 0xffff;
+        s->fuse_wo[SIFIVE_U_OTP_SERIAL_ADDR + 1] = 0xffff;
+    }
 
     /* Initialize file mmap and descriptor. */
     otp_mmap = NULL;
diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
index 1342bd7342..9c9c57f39e 100644
--- a/include/hw/riscv/sifive_u_otp.h
+++ b/include/hw/riscv/sifive_u_otp.h
@@ -77,6 +77,7 @@ typedef struct SiFiveUOTPState {
     uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
     /* config */
     uint32_t serial;
+    uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
 } SiFiveUOTPState;
 
 #endif /* HW_SIFIVE_U_OTP_H */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
  2020-07-24  9:51   ` Green Wan
@ 2020-07-24 14:20     ` Bin Meng
  -1 siblings, 0 replies; 12+ messages in thread
From: Bin Meng @ 2020-07-24 14:20 UTC (permalink / raw)
  To: Green Wan
  Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Paolo Bonzini, Palmer Dabbelt

Hi Green,

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

Instead of touching generic codes, could we add such property at the
board level?

>      QEMU_ARCH_ALL)
>  SRST

Regards,
Bin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
@ 2020-07-24 14:20     ` Bin Meng
  0 siblings, 0 replies; 12+ messages in thread
From: Bin Meng @ 2020-07-24 14:20 UTC (permalink / raw)
  To: Green Wan
  Cc: Palmer Dabbelt, Alistair Francis, Sagar Karandikar,
	Bastian Koppelmann, Paolo Bonzini, open list:RISC-V,
	qemu-devel@nongnu.org Developers

Hi Green,

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

Instead of touching generic codes, could we add such property at the
board level?

>      QEMU_ARCH_ALL)
>  SRST

Regards,
Bin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
  2020-07-24 14:20     ` Bin Meng
@ 2020-07-28  2:02       ` Green Wan
  -1 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-28  2:02 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Paolo Bonzini, Palmer Dabbelt

[-- Attachment #1: Type: text/plain, Size: 7401 bytes --]

Hi Bin,

Thanks for the reply.

I think we can add property to sifive_u_otp_properties[] (something like
below) and remove generic code dependency. What do you think of it?

@@ -243,6 +245,7 @@ static const MemoryRegionOps sifive_u_otp_ops = {

 static Property sifive_u_otp_properties[] = {
     DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0),
+    DEFINE_PROP_STRING("otp_file", SiFiveUOTPState, otp_file),
     DEFINE_PROP_END_OF_LIST(),
 };

 typedef struct SiFiveUOTPState {
     /*< private >*/
     SysBusDevice parent_obj;
@@ -77,6 +75,7 @@ typedef struct SiFiveUOTPState {
     uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
     /* config */
     uint32_t serial;
+    char *otp_file;
     uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
 } SiFiveUOTPState;

Regards,
Green


On Fri, Jul 24, 2020 at 10:20 PM Bin Meng <bmeng.cn@gmail.com> wrote:

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

[-- Attachment #2: Type: text/html, Size: 9910 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
@ 2020-07-28  2:02       ` Green Wan
  0 siblings, 0 replies; 12+ messages in thread
From: Green Wan @ 2020-07-28  2:02 UTC (permalink / raw)
  To: Bin Meng
  Cc: Palmer Dabbelt, Alistair Francis, Sagar Karandikar,
	Bastian Koppelmann, Paolo Bonzini, open list:RISC-V,
	qemu-devel@nongnu.org Developers

[-- Attachment #1: Type: text/plain, Size: 7401 bytes --]

Hi Bin,

Thanks for the reply.

I think we can add property to sifive_u_otp_properties[] (something like
below) and remove generic code dependency. What do you think of it?

@@ -243,6 +245,7 @@ static const MemoryRegionOps sifive_u_otp_ops = {

 static Property sifive_u_otp_properties[] = {
     DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0),
+    DEFINE_PROP_STRING("otp_file", SiFiveUOTPState, otp_file),
     DEFINE_PROP_END_OF_LIST(),
 };

 typedef struct SiFiveUOTPState {
     /*< private >*/
     SysBusDevice parent_obj;
@@ -77,6 +75,7 @@ typedef struct SiFiveUOTPState {
     uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
     /* config */
     uint32_t serial;
+    char *otp_file;
     uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
 } SiFiveUOTPState;

Regards,
Green


On Fri, Jul 24, 2020 at 10:20 PM Bin Meng <bmeng.cn@gmail.com> wrote:

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

[-- Attachment #2: Type: text/html, Size: 9910 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
  2020-07-28  2:02       ` Green Wan
@ 2020-07-28  8:28         ` Paolo Bonzini
  -1 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2020-07-28  8:28 UTC (permalink / raw)
  To: Green Wan, Bin Meng
  Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

On 28/07/20 04:02, Green Wan wrote:
> Hi Bin,
> 
> Thanks for the reply.
> 
> I think we can add property to sifive_u_otp_properties[] (something like
> below) and remove generic code dependency. What do you think of it?
> 
> @@ -243,6 +245,7 @@ static const MemoryRegionOps sifive_u_otp_ops = {
>  
>  static Property sifive_u_otp_properties[] = {
>      DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0),
> +    DEFINE_PROP_STRING("otp_file", SiFiveUOTPState, otp_file),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
>  typedef struct SiFiveUOTPState {
>      /*< private >*/
>      SysBusDevice parent_obj;
> @@ -77,6 +75,7 @@ typedef struct SiFiveUOTPState {
>      uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
>      /* config */
>      uint32_t serial;
> +    char *otp_file;
>      uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
>  } SiFiveUOTPState;

Yes, I agree.  (Also, dashes are preferred to underscores these days).

You can also add an alias to the machine so that you can access this
with "-machine otp-file=...".

Paolo

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
@ 2020-07-28  8:28         ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2020-07-28  8:28 UTC (permalink / raw)
  To: Green Wan, Bin Meng
  Cc: Palmer Dabbelt, Alistair Francis, Sagar Karandikar,
	Bastian Koppelmann, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On 28/07/20 04:02, Green Wan wrote:
> Hi Bin,
> 
> Thanks for the reply.
> 
> I think we can add property to sifive_u_otp_properties[] (something like
> below) and remove generic code dependency. What do you think of it?
> 
> @@ -243,6 +245,7 @@ static const MemoryRegionOps sifive_u_otp_ops = {
>  
>  static Property sifive_u_otp_properties[] = {
>      DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0),
> +    DEFINE_PROP_STRING("otp_file", SiFiveUOTPState, otp_file),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
>  typedef struct SiFiveUOTPState {
>      /*< private >*/
>      SysBusDevice parent_obj;
> @@ -77,6 +75,7 @@ typedef struct SiFiveUOTPState {
>      uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
>      /* config */
>      uint32_t serial;
> +    char *otp_file;
>      uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
>  } SiFiveUOTPState;

Yes, I agree.  (Also, dashes are preferred to underscores these days).

You can also add an alias to the machine so that you can access this
with "-machine otp-file=...".

Paolo

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-07-28  8:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24  9:51 [RFC PATCH 0/2] Add write-once and file-backed features to OTP Green Wan
2020-07-24  9:51 ` Green Wan
2020-07-24  9:51 ` [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option Green Wan
2020-07-24  9:51   ` Green Wan
2020-07-24 14:20   ` Bin Meng
2020-07-24 14:20     ` Bin Meng
2020-07-28  2:02     ` Green Wan
2020-07-28  2:02       ` Green Wan
2020-07-28  8:28       ` Paolo Bonzini
2020-07-28  8:28         ` Paolo Bonzini
2020-07-24  9:51 ` [RFC PATCH 2/2] hw/riscv: sifive_u: Add write-once protection Green Wan
2020-07-24  9:51   ` Green Wan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.