[RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option

Green Wan posted 2 patches 5 years, 6 months ago
Maintainers: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Paolo Bonzini <pbonzini@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>
There is a newer version of this series
[RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
Posted by Green Wan 5 years, 6 months ago
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


Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
Posted by Bin Meng 5 years, 6 months ago
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

Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
Posted by Green Wan 5 years, 6 months ago
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
>
Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option
Posted by Paolo Bonzini 5 years, 6 months ago
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
>