* [RFC PATCH v2 0/2] Add write-once and file-backed features to OTP
@ 2020-07-31 2:47 Green Wan
2020-07-31 2:47 ` [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP Green Wan
2020-07-31 2:47 ` [RFC PATCH v2 2/2] hw/riscv: sifive_u: Add write-once protection Green Wan
0 siblings, 2 replies; 7+ messages in thread
From: Green Wan @ 2020-07-31 2:47 UTC (permalink / raw)
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, qemu-devel,
Green Wan, Alistair Francis, Palmer Dabbelt, bmeng.cn
First patch is to add file-backed implementation to allow users to assign
an OTP image file to machine. Users can assign the property, "otp-file",
to machine to enable it. File-backed feature is set to "NULL" string in
default. Any filename other than "NULL" is used as OTP image file.
For example, '-M sifive_u,otp-file="otp.img"'
To keep data up-to-date due to an unexpected crash or CTRL+a-x exit, every
read/write command to OTP memory involves file open, mmap and close
operation to the image file.
Second patch is to add 'write-once' feature to block second write to the
OTP memory. Only keep the 'written' state for non-control register range
from 0x38 to 16KB.
Tested on sifive_u for both qemu and u-boot.
Green Wan (2):
hw/riscv: sifive_u: Add file-backed OTP.
hw/riscv: sifive_u: Add write-once protection.
hw/riscv/sifive_u.c | 26 ++++++++
hw/riscv/sifive_u_otp.c | 103 ++++++++++++++++++++++++++++++++
include/hw/riscv/sifive_u.h | 2 +
include/hw/riscv/sifive_u_otp.h | 2 +
4 files changed, 133 insertions(+)
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP.
2020-07-31 2:47 [RFC PATCH v2 0/2] Add write-once and file-backed features to OTP Green Wan
@ 2020-07-31 2:47 ` Green Wan
2020-08-10 22:13 ` Alistair Francis
2020-07-31 2:47 ` [RFC PATCH v2 2/2] hw/riscv: sifive_u: Add write-once protection Green Wan
1 sibling, 1 reply; 7+ messages in thread
From: Green Wan @ 2020-07-31 2:47 UTC (permalink / raw)
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, qemu-devel,
Green Wan, Alistair Francis, Palmer Dabbelt, bmeng.cn
Add a file-backed implementation for OTP of sifive_u machine. The
machine property for file-backed is disabled in default. 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.c | 26 +++++++++++
hw/riscv/sifive_u_otp.c | 83 +++++++++++++++++++++++++++++++++
include/hw/riscv/sifive_u.h | 2 +
include/hw/riscv/sifive_u_otp.h | 1 +
4 files changed, 112 insertions(+)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e5682c38a9..c818496918 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -87,6 +87,7 @@ static const struct MemmapEntry {
};
#define OTP_SERIAL 1
+#define OTP_FILE "NULL"
#define GEM_REVISION 0x10070109
static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
@@ -387,6 +388,8 @@ static void sifive_u_machine_init(MachineState *machine)
object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RISCV_U_SOC);
object_property_set_uint(OBJECT(&s->soc), "serial", s->serial,
&error_abort);
+ object_property_set_str(OBJECT(&s->soc), "otp-file", s->otp_file,
+ &error_abort);
qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
/* register RAM */
@@ -526,6 +529,21 @@ static void sifive_u_machine_set_uint32_prop(Object *obj, Visitor *v,
visit_type_uint32(v, name, (uint32_t *)opaque, errp);
}
+static void sifive_u_machine_get_str_prop(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ visit_type_str(v, name, (char **)opaque, errp);
+}
+
+static void sifive_u_machine_set_str_prop(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ visit_type_str(v, name, (char **)opaque, errp);
+}
+
+
static void sifive_u_machine_instance_init(Object *obj)
{
SiFiveUState *s = RISCV_U_MACHINE(obj);
@@ -551,6 +569,12 @@ static void sifive_u_machine_instance_init(Object *obj)
sifive_u_machine_get_uint32_prop,
sifive_u_machine_set_uint32_prop, NULL, &s->serial);
object_property_set_description(obj, "serial", "Board serial number");
+
+ s->otp_file = (char *)OTP_FILE;
+ object_property_add(obj, "otp-file", "string",
+ sifive_u_machine_get_str_prop,
+ sifive_u_machine_set_str_prop, NULL, &s->otp_file);
+ object_property_set_description(obj, "otp-file", "file-backed otp file");
}
static void sifive_u_machine_class_init(ObjectClass *oc, void *data)
@@ -709,6 +733,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
}
qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
+ qdev_prop_set_string(DEVICE(&s->otp), "otp-file", s->otp_file);
if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
return;
}
@@ -737,6 +762,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
static Property sifive_u_soc_props[] = {
DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL),
+ DEFINE_PROP_STRING("otp-file", SiFiveUSoCState, otp_file),
DEFINE_PROP_END_OF_LIST()
};
diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
index f6ecbaa2ca..e359f30fdb 100644
--- a/hw/riscv/sifive_u_otp.c
+++ b/hw/riscv/sifive_u_otp.c
@@ -24,6 +24,62 @@
#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 int32_t sifive_u_otp_backed_open(const char *filename, int32_t *fd,
+ uint32_t **map)
+{
+ /* open and mmap OTP image file */
+ if (filename && strcmp(filename, "NULL") != 0) {
+ *fd = open(filename, O_RDWR);
+
+ if (*fd < 0) {
+ qemu_log_mask(LOG_TRACE,
+ TRACE_PREFIX "Warning: can't open otp file<%s>\n",
+ filename);
+ return -1;
+ } else {
+
+ *map = (unsigned int *)mmap(0,
+ SIFIVE_FU540_OTP_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_FILE | MAP_SHARED,
+ *fd,
+ 0);
+
+ if (*map == MAP_FAILED) {
+ qemu_log_mask(LOG_TRACE,
+ TRACE_PREFIX "Warning: can't mmap otp file<%s>\n",
+ filename);
+ close(*fd);
+ return -2;
+ }
+ }
+ } else {
+ /* filename is 'NULL' */
+ return -3;
+ }
+
+ return 0;
+}
+
+static int32_t sifive_u_otp_backed_close(int fd, unsigned int *map)
+{
+ munmap(map, SIFIVE_FU540_OTP_SIZE);
+ close(fd);
+
+ return 0;
+}
static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
{
@@ -46,6 +102,20 @@ 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)) {
+
+ int32_t fd;
+ uint32_t *map;
+ uint64_t val;
+
+ /* open and mmap OTP image file */
+ if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
+ val = (uint64_t)(map[s->pa]);
+
+ /* close and unmmap */
+ sifive_u_otp_backed_close(fd, map);
+ return val;
+ }
+
return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
} else {
return 0xff;
@@ -78,6 +148,8 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
{
SiFiveUOTPState *s = opaque;
uint32_t val32 = (uint32_t)val64;
+ int32_t fd;
+ uint32_t *map;
switch (addr) {
case SIFIVE_U_OTP_PA:
@@ -123,6 +195,16 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
s->ptrim = val32;
break;
case SIFIVE_U_OTP_PWE:
+ /* open and mmap OTP image file */
+ if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
+ /* store value */
+ map[s->pa] &= ~(s->pdin << s->paio);
+ map[s->pa] |= (s->pdin << s->paio);
+
+ /* close and unmmap */
+ sifive_u_otp_backed_close(fd, map);
+ }
+
s->pwe = val32;
break;
default:
@@ -143,6 +225,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(),
};
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index aba4d0181f..966723da1d 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -46,6 +46,7 @@ typedef struct SiFiveUSoCState {
CadenceGEMState gem;
uint32_t serial;
+ char *otp_file;
} SiFiveUSoCState;
#define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u")
@@ -65,6 +66,7 @@ typedef struct SiFiveUState {
bool start_in_flash;
uint32_t msel;
uint32_t serial;
+ char *otp_file;
} SiFiveUState;
enum {
diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
index 639297564a..f3d71ce82d 100644
--- a/include/hw/riscv/sifive_u_otp.h
+++ b/include/hw/riscv/sifive_u_otp.h
@@ -75,6 +75,7 @@ typedef struct SiFiveUOTPState {
uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
/* config */
uint32_t serial;
+ char *otp_file;
} SiFiveUOTPState;
#endif /* HW_SIFIVE_U_OTP_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH v2 2/2] hw/riscv: sifive_u: Add write-once protection.
2020-07-31 2:47 [RFC PATCH v2 0/2] Add write-once and file-backed features to OTP Green Wan
2020-07-31 2:47 ` [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP Green Wan
@ 2020-07-31 2:47 ` Green Wan
1 sibling, 0 replies; 7+ messages in thread
From: Green Wan @ 2020-07-31 2:47 UTC (permalink / raw)
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, qemu-devel,
Green Wan, Alistair Francis, 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 | 20 ++++++++++++++++++++
include/hw/riscv/sifive_u_otp.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
index e359f30fdb..a793093d47 100644
--- a/hw/riscv/sifive_u_otp.c
+++ b/hw/riscv/sifive_u_otp.c
@@ -35,6 +35,11 @@
#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 int32_t sifive_u_otp_backed_open(const char *filename, int32_t *fd,
uint32_t **map)
@@ -195,6 +200,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);
+ }
+ }
+
/* open and mmap OTP image file */
if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
/* store value */
@@ -248,6 +265,9 @@ static void sifive_u_otp_reset(DeviceState *dev)
/* Make a valid content of serial number */
s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
+
+ /* Initialize write-once map */
+ memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));
}
static void sifive_u_otp_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
index f3d71ce82d..4c6ac2e75e 100644
--- a/include/hw/riscv/sifive_u_otp.h
+++ b/include/hw/riscv/sifive_u_otp.h
@@ -73,6 +73,7 @@ typedef struct SiFiveUOTPState {
uint32_t ptrim;
uint32_t pwe;
uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
+ uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
/* config */
uint32_t serial;
char *otp_file;
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP.
2020-07-31 2:47 ` [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP Green Wan
@ 2020-08-10 22:13 ` Alistair Francis
2020-08-13 4:12 ` Green Wan
0 siblings, 1 reply; 7+ messages in thread
From: Alistair Francis @ 2020-08-10 22:13 UTC (permalink / raw)
To: Green Wan
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt, Bin Meng
,()On Thu, Jul 30, 2020 at 7:49 PM Green Wan <green.wan@sifive.com> wrote:
>
> Add a file-backed implementation for OTP of sifive_u machine. The
> machine property for file-backed is disabled in default. Do file
> open, mmap and close for every OTP read/write in case keep the
> update-to-date snapshot of OTP.
I don't think this is the correct way to write to the file.
QEMU has backends that should do this for you. For example QEMU
includes the -blockdev/-driver or -mtdblock command line arguments.
This implementation should look more like an SD card in terms of
interface. You will probably want to call drive_get_next() (probably
with IF_MTD, but that's up to you).
The hw/arm/xlnx-zcu102.c file has a good example of attaching an SD
card by setting the drive property.
Alistair
>
> Signed-off-by: Green Wan <green.wan@sifive.com>
> ---
> hw/riscv/sifive_u.c | 26 +++++++++++
> hw/riscv/sifive_u_otp.c | 83 +++++++++++++++++++++++++++++++++
> include/hw/riscv/sifive_u.h | 2 +
> include/hw/riscv/sifive_u_otp.h | 1 +
> 4 files changed, 112 insertions(+)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index e5682c38a9..c818496918 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -87,6 +87,7 @@ static const struct MemmapEntry {
> };
>
> #define OTP_SERIAL 1
> +#define OTP_FILE "NULL"
> #define GEM_REVISION 0x10070109
>
> static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
> @@ -387,6 +388,8 @@ static void sifive_u_machine_init(MachineState *machine)
> object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RISCV_U_SOC);
> object_property_set_uint(OBJECT(&s->soc), "serial", s->serial,
> &error_abort);
> + object_property_set_str(OBJECT(&s->soc), "otp-file", s->otp_file,
> + &error_abort);
> qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
>
> /* register RAM */
> @@ -526,6 +529,21 @@ static void sifive_u_machine_set_uint32_prop(Object *obj, Visitor *v,
> visit_type_uint32(v, name, (uint32_t *)opaque, errp);
> }
>
> +static void sifive_u_machine_get_str_prop(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + visit_type_str(v, name, (char **)opaque, errp);
> +}
> +
> +static void sifive_u_machine_set_str_prop(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + visit_type_str(v, name, (char **)opaque, errp);
> +}
> +
> +
> static void sifive_u_machine_instance_init(Object *obj)
> {
> SiFiveUState *s = RISCV_U_MACHINE(obj);
> @@ -551,6 +569,12 @@ static void sifive_u_machine_instance_init(Object *obj)
> sifive_u_machine_get_uint32_prop,
> sifive_u_machine_set_uint32_prop, NULL, &s->serial);
> object_property_set_description(obj, "serial", "Board serial number");
> +
> + s->otp_file = (char *)OTP_FILE;
> + object_property_add(obj, "otp-file", "string",
> + sifive_u_machine_get_str_prop,
> + sifive_u_machine_set_str_prop, NULL, &s->otp_file);
> + object_property_set_description(obj, "otp-file", "file-backed otp file");
> }
>
> static void sifive_u_machine_class_init(ObjectClass *oc, void *data)
> @@ -709,6 +733,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
> }
>
> qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
> + qdev_prop_set_string(DEVICE(&s->otp), "otp-file", s->otp_file);
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
> return;
> }
> @@ -737,6 +762,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
>
> static Property sifive_u_soc_props[] = {
> DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL),
> + DEFINE_PROP_STRING("otp-file", SiFiveUSoCState, otp_file),
> DEFINE_PROP_END_OF_LIST()
> };
>
> diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
> index f6ecbaa2ca..e359f30fdb 100644
> --- a/hw/riscv/sifive_u_otp.c
> +++ b/hw/riscv/sifive_u_otp.c
> @@ -24,6 +24,62 @@
> #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 int32_t sifive_u_otp_backed_open(const char *filename, int32_t *fd,
> + uint32_t **map)
> +{
> + /* open and mmap OTP image file */
> + if (filename && strcmp(filename, "NULL") != 0) {
> + *fd = open(filename, O_RDWR);
> +
> + if (*fd < 0) {
> + qemu_log_mask(LOG_TRACE,
> + TRACE_PREFIX "Warning: can't open otp file<%s>\n",
> + filename);
> + return -1;
> + } else {
> +
> + *map = (unsigned int *)mmap(0,
> + SIFIVE_FU540_OTP_SIZE,
> + PROT_READ | PROT_WRITE | PROT_EXEC,
> + MAP_FILE | MAP_SHARED,
> + *fd,
> + 0);
> +
> + if (*map == MAP_FAILED) {
> + qemu_log_mask(LOG_TRACE,
> + TRACE_PREFIX "Warning: can't mmap otp file<%s>\n",
> + filename);
> + close(*fd);
> + return -2;
> + }
> + }
> + } else {
> + /* filename is 'NULL' */
> + return -3;
> + }
> +
> + return 0;
> +}
> +
> +static int32_t sifive_u_otp_backed_close(int fd, unsigned int *map)
> +{
> + munmap(map, SIFIVE_FU540_OTP_SIZE);
> + close(fd);
> +
> + return 0;
> +}
>
> static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
> {
> @@ -46,6 +102,20 @@ 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)) {
> +
> + int32_t fd;
> + uint32_t *map;
> + uint64_t val;
> +
> + /* open and mmap OTP image file */
> + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
> + val = (uint64_t)(map[s->pa]);
> +
> + /* close and unmmap */
> + sifive_u_otp_backed_close(fd, map);
> + return val;
> + }
> +
> return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
> } else {
> return 0xff;
> @@ -78,6 +148,8 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
> {
> SiFiveUOTPState *s = opaque;
> uint32_t val32 = (uint32_t)val64;
> + int32_t fd;
> + uint32_t *map;
>
> switch (addr) {
> case SIFIVE_U_OTP_PA:
> @@ -123,6 +195,16 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
> s->ptrim = val32;
> break;
> case SIFIVE_U_OTP_PWE:
> + /* open and mmap OTP image file */
> + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
> + /* store value */
> + map[s->pa] &= ~(s->pdin << s->paio);
> + map[s->pa] |= (s->pdin << s->paio);
> +
> + /* close and unmmap */
> + sifive_u_otp_backed_close(fd, map);
> + }
> +
> s->pwe = val32;
> break;
> default:
> @@ -143,6 +225,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(),
> };
>
> diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> index aba4d0181f..966723da1d 100644
> --- a/include/hw/riscv/sifive_u.h
> +++ b/include/hw/riscv/sifive_u.h
> @@ -46,6 +46,7 @@ typedef struct SiFiveUSoCState {
> CadenceGEMState gem;
>
> uint32_t serial;
> + char *otp_file;
> } SiFiveUSoCState;
>
> #define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u")
> @@ -65,6 +66,7 @@ typedef struct SiFiveUState {
> bool start_in_flash;
> uint32_t msel;
> uint32_t serial;
> + char *otp_file;
> } SiFiveUState;
>
> enum {
> diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
> index 639297564a..f3d71ce82d 100644
> --- a/include/hw/riscv/sifive_u_otp.h
> +++ b/include/hw/riscv/sifive_u_otp.h
> @@ -75,6 +75,7 @@ typedef struct SiFiveUOTPState {
> uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
> /* config */
> uint32_t serial;
> + char *otp_file;
> } SiFiveUOTPState;
>
> #endif /* HW_SIFIVE_U_OTP_H */
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP.
2020-08-10 22:13 ` Alistair Francis
@ 2020-08-13 4:12 ` Green Wan
2020-08-13 21:24 ` Alistair Francis
0 siblings, 1 reply; 7+ messages in thread
From: Green Wan @ 2020-08-13 4:12 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt, Bin Meng
[-- Attachment #1: Type: text/plain, Size: 10456 bytes --]
Hi Alistair,
Thanks for the feedback and tips. Not sure whether I get it right. I gave a
try with -drive and -device options as below.
$ qemu-system-riscv64 -M sifive_u -drive if=none,format=raw,file=otp.img
-device riscv.sifive.u.otp
qemu-system-riscv64: -device riscv.sifive.u.otp: Parameter 'driver' expects
pluggable device type
Then I dump "info qtree". The device, "riscv.sifive.u.otp", belongs to
'System' bus. (dump list by 'info qdm') and all devices on 'System' bus
seem not available with "-device". Any suggestions for specifying the
device?
Thanks,
- Green
On Tue, Aug 11, 2020 at 6:24 AM Alistair Francis <alistair23@gmail.com>
wrote:
> ,()On Thu, Jul 30, 2020 at 7:49 PM Green Wan <green.wan@sifive.com> wrote:
> >
> > Add a file-backed implementation for OTP of sifive_u machine. The
> > machine property for file-backed is disabled in default. Do file
> > open, mmap and close for every OTP read/write in case keep the
> > update-to-date snapshot of OTP.
>
> I don't think this is the correct way to write to the file.
>
> QEMU has backends that should do this for you. For example QEMU
> includes the -blockdev/-driver or -mtdblock command line arguments.
>
> This implementation should look more like an SD card in terms of
> interface. You will probably want to call drive_get_next() (probably
> with IF_MTD, but that's up to you).
>
> The hw/arm/xlnx-zcu102.c file has a good example of attaching an SD
> card by setting the drive property.
>
> Alistair
>
> >
> > Signed-off-by: Green Wan <green.wan@sifive.com>
> > ---
> > hw/riscv/sifive_u.c | 26 +++++++++++
> > hw/riscv/sifive_u_otp.c | 83 +++++++++++++++++++++++++++++++++
> > include/hw/riscv/sifive_u.h | 2 +
> > include/hw/riscv/sifive_u_otp.h | 1 +
> > 4 files changed, 112 insertions(+)
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index e5682c38a9..c818496918 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -87,6 +87,7 @@ static const struct MemmapEntry {
> > };
> >
> > #define OTP_SERIAL 1
> > +#define OTP_FILE "NULL"
> > #define GEM_REVISION 0x10070109
> >
> > static void create_fdt(SiFiveUState *s, const struct MemmapEntry
> *memmap,
> > @@ -387,6 +388,8 @@ static void sifive_u_machine_init(MachineState
> *machine)
> > object_initialize_child(OBJECT(machine), "soc", &s->soc,
> TYPE_RISCV_U_SOC);
> > object_property_set_uint(OBJECT(&s->soc), "serial", s->serial,
> > &error_abort);
> > + object_property_set_str(OBJECT(&s->soc), "otp-file", s->otp_file,
> > + &error_abort);
> > qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
> >
> > /* register RAM */
> > @@ -526,6 +529,21 @@ static void sifive_u_machine_set_uint32_prop(Object
> *obj, Visitor *v,
> > visit_type_uint32(v, name, (uint32_t *)opaque, errp);
> > }
> >
> > +static void sifive_u_machine_get_str_prop(Object *obj, Visitor *v,
> > + const char *name, void
> *opaque,
> > + Error **errp)
> > +{
> > + visit_type_str(v, name, (char **)opaque, errp);
> > +}
> > +
> > +static void sifive_u_machine_set_str_prop(Object *obj, Visitor *v,
> > + const char *name, void
> *opaque,
> > + Error **errp)
> > +{
> > + visit_type_str(v, name, (char **)opaque, errp);
> > +}
> > +
> > +
> > static void sifive_u_machine_instance_init(Object *obj)
> > {
> > SiFiveUState *s = RISCV_U_MACHINE(obj);
> > @@ -551,6 +569,12 @@ static void sifive_u_machine_instance_init(Object
> *obj)
> > sifive_u_machine_get_uint32_prop,
> > sifive_u_machine_set_uint32_prop, NULL,
> &s->serial);
> > object_property_set_description(obj, "serial", "Board serial
> number");
> > +
> > + s->otp_file = (char *)OTP_FILE;
> > + object_property_add(obj, "otp-file", "string",
> > + sifive_u_machine_get_str_prop,
> > + sifive_u_machine_set_str_prop, NULL,
> &s->otp_file);
> > + object_property_set_description(obj, "otp-file", "file-backed otp
> file");
> > }
> >
> > static void sifive_u_machine_class_init(ObjectClass *oc, void *data)
> > @@ -709,6 +733,7 @@ static void sifive_u_soc_realize(DeviceState *dev,
> Error **errp)
> > }
> >
> > qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
> > + qdev_prop_set_string(DEVICE(&s->otp), "otp-file", s->otp_file);
> > if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
> > return;
> > }
> > @@ -737,6 +762,7 @@ static void sifive_u_soc_realize(DeviceState *dev,
> Error **errp)
> >
> > static Property sifive_u_soc_props[] = {
> > DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL),
> > + DEFINE_PROP_STRING("otp-file", SiFiveUSoCState, otp_file),
> > DEFINE_PROP_END_OF_LIST()
> > };
> >
> > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
> > index f6ecbaa2ca..e359f30fdb 100644
> > --- a/hw/riscv/sifive_u_otp.c
> > +++ b/hw/riscv/sifive_u_otp.c
> > @@ -24,6 +24,62 @@
> > #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 int32_t sifive_u_otp_backed_open(const char *filename, int32_t
> *fd,
> > + uint32_t **map)
> > +{
> > + /* open and mmap OTP image file */
> > + if (filename && strcmp(filename, "NULL") != 0) {
> > + *fd = open(filename, O_RDWR);
> > +
> > + if (*fd < 0) {
> > + qemu_log_mask(LOG_TRACE,
> > + TRACE_PREFIX "Warning: can't open otp
> file<%s>\n",
> > + filename);
> > + return -1;
> > + } else {
> > +
> > + *map = (unsigned int *)mmap(0,
> > + SIFIVE_FU540_OTP_SIZE,
> > + PROT_READ | PROT_WRITE |
> PROT_EXEC,
> > + MAP_FILE | MAP_SHARED,
> > + *fd,
> > + 0);
> > +
> > + if (*map == MAP_FAILED) {
> > + qemu_log_mask(LOG_TRACE,
> > + TRACE_PREFIX "Warning: can't mmap otp
> file<%s>\n",
> > + filename);
> > + close(*fd);
> > + return -2;
> > + }
> > + }
> > + } else {
> > + /* filename is 'NULL' */
> > + return -3;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int32_t sifive_u_otp_backed_close(int fd, unsigned int *map)
> > +{
> > + munmap(map, SIFIVE_FU540_OTP_SIZE);
> > + close(fd);
> > +
> > + return 0;
> > +}
> >
> > static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned
> int size)
> > {
> > @@ -46,6 +102,20 @@ 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)) {
> > +
> > + int32_t fd;
> > + uint32_t *map;
> > + uint64_t val;
> > +
> > + /* open and mmap OTP image file */
> > + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
> > + val = (uint64_t)(map[s->pa]);
> > +
> > + /* close and unmmap */
> > + sifive_u_otp_backed_close(fd, map);
> > + return val;
> > + }
> > +
> > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
> > } else {
> > return 0xff;
> > @@ -78,6 +148,8 @@ static void sifive_u_otp_write(void *opaque, hwaddr
> addr,
> > {
> > SiFiveUOTPState *s = opaque;
> > uint32_t val32 = (uint32_t)val64;
> > + int32_t fd;
> > + uint32_t *map;
> >
> > switch (addr) {
> > case SIFIVE_U_OTP_PA:
> > @@ -123,6 +195,16 @@ static void sifive_u_otp_write(void *opaque, hwaddr
> addr,
> > s->ptrim = val32;
> > break;
> > case SIFIVE_U_OTP_PWE:
> > + /* open and mmap OTP image file */
> > + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
> > + /* store value */
> > + map[s->pa] &= ~(s->pdin << s->paio);
> > + map[s->pa] |= (s->pdin << s->paio);
> > +
> > + /* close and unmmap */
> > + sifive_u_otp_backed_close(fd, map);
> > + }
> > +
> > s->pwe = val32;
> > break;
> > default:
> > @@ -143,6 +225,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(),
> > };
> >
> > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> > index aba4d0181f..966723da1d 100644
> > --- a/include/hw/riscv/sifive_u.h
> > +++ b/include/hw/riscv/sifive_u.h
> > @@ -46,6 +46,7 @@ typedef struct SiFiveUSoCState {
> > CadenceGEMState gem;
> >
> > uint32_t serial;
> > + char *otp_file;
> > } SiFiveUSoCState;
> >
> > #define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u")
> > @@ -65,6 +66,7 @@ typedef struct SiFiveUState {
> > bool start_in_flash;
> > uint32_t msel;
> > uint32_t serial;
> > + char *otp_file;
> > } SiFiveUState;
> >
> > enum {
> > diff --git a/include/hw/riscv/sifive_u_otp.h
> b/include/hw/riscv/sifive_u_otp.h
> > index 639297564a..f3d71ce82d 100644
> > --- a/include/hw/riscv/sifive_u_otp.h
> > +++ b/include/hw/riscv/sifive_u_otp.h
> > @@ -75,6 +75,7 @@ typedef struct SiFiveUOTPState {
> > uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
> > /* config */
> > uint32_t serial;
> > + char *otp_file;
> > } SiFiveUOTPState;
> >
> > #endif /* HW_SIFIVE_U_OTP_H */
> > --
> > 2.17.1
> >
> >
>
[-- Attachment #2: Type: text/html, Size: 13772 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP.
2020-08-13 4:12 ` Green Wan
@ 2020-08-13 21:24 ` Alistair Francis
2020-08-18 17:12 ` Green Wan
0 siblings, 1 reply; 7+ messages in thread
From: Alistair Francis @ 2020-08-13 21:24 UTC (permalink / raw)
To: Green Wan
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt, Bin Meng
On Wed, Aug 12, 2020 at 9:12 PM Green Wan <green.wan@sifive.com> wrote:
>
> Hi Alistair,
>
> Thanks for the feedback and tips. Not sure whether I get it right. I gave a try with -drive and -device options as below.
>
> $ qemu-system-riscv64 -M sifive_u -drive if=none,format=raw,file=otp.img -device riscv.sifive.u.otp
> qemu-system-riscv64: -device riscv.sifive.u.otp: Parameter 'driver' expects pluggable device type
You don't need the -device, -drive should be enough (and then the OTP
device needs to be re-written to support it).
Alistair
>
> Then I dump "info qtree". The device, "riscv.sifive.u.otp", belongs to 'System' bus. (dump list by 'info qdm') and all devices on 'System' bus seem not available with "-device". Any suggestions for specifying the device?
>
> Thanks,
> - Green
>
> On Tue, Aug 11, 2020 at 6:24 AM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> ,()On Thu, Jul 30, 2020 at 7:49 PM Green Wan <green.wan@sifive.com> wrote:
>> >
>> > Add a file-backed implementation for OTP of sifive_u machine. The
>> > machine property for file-backed is disabled in default. Do file
>> > open, mmap and close for every OTP read/write in case keep the
>> > update-to-date snapshot of OTP.
>>
>> I don't think this is the correct way to write to the file.
>>
>> QEMU has backends that should do this for you. For example QEMU
>> includes the -blockdev/-driver or -mtdblock command line arguments.
>>
>> This implementation should look more like an SD card in terms of
>> interface. You will probably want to call drive_get_next() (probably
>> with IF_MTD, but that's up to you).
>>
>> The hw/arm/xlnx-zcu102.c file has a good example of attaching an SD
>> card by setting the drive property.
>>
>> Alistair
>>
>> >
>> > Signed-off-by: Green Wan <green.wan@sifive.com>
>> > ---
>> > hw/riscv/sifive_u.c | 26 +++++++++++
>> > hw/riscv/sifive_u_otp.c | 83 +++++++++++++++++++++++++++++++++
>> > include/hw/riscv/sifive_u.h | 2 +
>> > include/hw/riscv/sifive_u_otp.h | 1 +
>> > 4 files changed, 112 insertions(+)
>> >
>> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
>> > index e5682c38a9..c818496918 100644
>> > --- a/hw/riscv/sifive_u.c
>> > +++ b/hw/riscv/sifive_u.c
>> > @@ -87,6 +87,7 @@ static const struct MemmapEntry {
>> > };
>> >
>> > #define OTP_SERIAL 1
>> > +#define OTP_FILE "NULL"
>> > #define GEM_REVISION 0x10070109
>> >
>> > static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
>> > @@ -387,6 +388,8 @@ static void sifive_u_machine_init(MachineState *machine)
>> > object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RISCV_U_SOC);
>> > object_property_set_uint(OBJECT(&s->soc), "serial", s->serial,
>> > &error_abort);
>> > + object_property_set_str(OBJECT(&s->soc), "otp-file", s->otp_file,
>> > + &error_abort);
>> > qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
>> >
>> > /* register RAM */
>> > @@ -526,6 +529,21 @@ static void sifive_u_machine_set_uint32_prop(Object *obj, Visitor *v,
>> > visit_type_uint32(v, name, (uint32_t *)opaque, errp);
>> > }
>> >
>> > +static void sifive_u_machine_get_str_prop(Object *obj, Visitor *v,
>> > + const char *name, void *opaque,
>> > + Error **errp)
>> > +{
>> > + visit_type_str(v, name, (char **)opaque, errp);
>> > +}
>> > +
>> > +static void sifive_u_machine_set_str_prop(Object *obj, Visitor *v,
>> > + const char *name, void *opaque,
>> > + Error **errp)
>> > +{
>> > + visit_type_str(v, name, (char **)opaque, errp);
>> > +}
>> > +
>> > +
>> > static void sifive_u_machine_instance_init(Object *obj)
>> > {
>> > SiFiveUState *s = RISCV_U_MACHINE(obj);
>> > @@ -551,6 +569,12 @@ static void sifive_u_machine_instance_init(Object *obj)
>> > sifive_u_machine_get_uint32_prop,
>> > sifive_u_machine_set_uint32_prop, NULL, &s->serial);
>> > object_property_set_description(obj, "serial", "Board serial number");
>> > +
>> > + s->otp_file = (char *)OTP_FILE;
>> > + object_property_add(obj, "otp-file", "string",
>> > + sifive_u_machine_get_str_prop,
>> > + sifive_u_machine_set_str_prop, NULL, &s->otp_file);
>> > + object_property_set_description(obj, "otp-file", "file-backed otp file");
>> > }
>> >
>> > static void sifive_u_machine_class_init(ObjectClass *oc, void *data)
>> > @@ -709,6 +733,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
>> > }
>> >
>> > qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
>> > + qdev_prop_set_string(DEVICE(&s->otp), "otp-file", s->otp_file);
>> > if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
>> > return;
>> > }
>> > @@ -737,6 +762,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
>> >
>> > static Property sifive_u_soc_props[] = {
>> > DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL),
>> > + DEFINE_PROP_STRING("otp-file", SiFiveUSoCState, otp_file),
>> > DEFINE_PROP_END_OF_LIST()
>> > };
>> >
>> > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
>> > index f6ecbaa2ca..e359f30fdb 100644
>> > --- a/hw/riscv/sifive_u_otp.c
>> > +++ b/hw/riscv/sifive_u_otp.c
>> > @@ -24,6 +24,62 @@
>> > #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 int32_t sifive_u_otp_backed_open(const char *filename, int32_t *fd,
>> > + uint32_t **map)
>> > +{
>> > + /* open and mmap OTP image file */
>> > + if (filename && strcmp(filename, "NULL") != 0) {
>> > + *fd = open(filename, O_RDWR);
>> > +
>> > + if (*fd < 0) {
>> > + qemu_log_mask(LOG_TRACE,
>> > + TRACE_PREFIX "Warning: can't open otp file<%s>\n",
>> > + filename);
>> > + return -1;
>> > + } else {
>> > +
>> > + *map = (unsigned int *)mmap(0,
>> > + SIFIVE_FU540_OTP_SIZE,
>> > + PROT_READ | PROT_WRITE | PROT_EXEC,
>> > + MAP_FILE | MAP_SHARED,
>> > + *fd,
>> > + 0);
>> > +
>> > + if (*map == MAP_FAILED) {
>> > + qemu_log_mask(LOG_TRACE,
>> > + TRACE_PREFIX "Warning: can't mmap otp file<%s>\n",
>> > + filename);
>> > + close(*fd);
>> > + return -2;
>> > + }
>> > + }
>> > + } else {
>> > + /* filename is 'NULL' */
>> > + return -3;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int32_t sifive_u_otp_backed_close(int fd, unsigned int *map)
>> > +{
>> > + munmap(map, SIFIVE_FU540_OTP_SIZE);
>> > + close(fd);
>> > +
>> > + return 0;
>> > +}
>> >
>> > static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
>> > {
>> > @@ -46,6 +102,20 @@ 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)) {
>> > +
>> > + int32_t fd;
>> > + uint32_t *map;
>> > + uint64_t val;
>> > +
>> > + /* open and mmap OTP image file */
>> > + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
>> > + val = (uint64_t)(map[s->pa]);
>> > +
>> > + /* close and unmmap */
>> > + sifive_u_otp_backed_close(fd, map);
>> > + return val;
>> > + }
>> > +
>> > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
>> > } else {
>> > return 0xff;
>> > @@ -78,6 +148,8 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
>> > {
>> > SiFiveUOTPState *s = opaque;
>> > uint32_t val32 = (uint32_t)val64;
>> > + int32_t fd;
>> > + uint32_t *map;
>> >
>> > switch (addr) {
>> > case SIFIVE_U_OTP_PA:
>> > @@ -123,6 +195,16 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
>> > s->ptrim = val32;
>> > break;
>> > case SIFIVE_U_OTP_PWE:
>> > + /* open and mmap OTP image file */
>> > + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
>> > + /* store value */
>> > + map[s->pa] &= ~(s->pdin << s->paio);
>> > + map[s->pa] |= (s->pdin << s->paio);
>> > +
>> > + /* close and unmmap */
>> > + sifive_u_otp_backed_close(fd, map);
>> > + }
>> > +
>> > s->pwe = val32;
>> > break;
>> > default:
>> > @@ -143,6 +225,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(),
>> > };
>> >
>> > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
>> > index aba4d0181f..966723da1d 100644
>> > --- a/include/hw/riscv/sifive_u.h
>> > +++ b/include/hw/riscv/sifive_u.h
>> > @@ -46,6 +46,7 @@ typedef struct SiFiveUSoCState {
>> > CadenceGEMState gem;
>> >
>> > uint32_t serial;
>> > + char *otp_file;
>> > } SiFiveUSoCState;
>> >
>> > #define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u")
>> > @@ -65,6 +66,7 @@ typedef struct SiFiveUState {
>> > bool start_in_flash;
>> > uint32_t msel;
>> > uint32_t serial;
>> > + char *otp_file;
>> > } SiFiveUState;
>> >
>> > enum {
>> > diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
>> > index 639297564a..f3d71ce82d 100644
>> > --- a/include/hw/riscv/sifive_u_otp.h
>> > +++ b/include/hw/riscv/sifive_u_otp.h
>> > @@ -75,6 +75,7 @@ typedef struct SiFiveUOTPState {
>> > uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
>> > /* config */
>> > uint32_t serial;
>> > + char *otp_file;
>> > } SiFiveUOTPState;
>> >
>> > #endif /* HW_SIFIVE_U_OTP_H */
>> > --
>> > 2.17.1
>> >
>> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP.
2020-08-13 21:24 ` Alistair Francis
@ 2020-08-18 17:12 ` Green Wan
0 siblings, 0 replies; 7+ messages in thread
From: Green Wan @ 2020-08-18 17:12 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt, Bin Meng
[-- Attachment #1: Type: text/plain, Size: 11730 bytes --]
Hi Alistair,
You're right. I can get drive without -device. Now it looks much better.
Sent v3 patch.
Thanks a lot,
- Green
On Fri, Aug 14, 2020 at 5:34 AM Alistair Francis <alistair23@gmail.com>
wrote:
> On Wed, Aug 12, 2020 at 9:12 PM Green Wan <green.wan@sifive.com> wrote:
> >
> > Hi Alistair,
> >
> > Thanks for the feedback and tips. Not sure whether I get it right. I
> gave a try with -drive and -device options as below.
> >
> > $ qemu-system-riscv64 -M sifive_u -drive if=none,format=raw,file=otp.img
> -device riscv.sifive.u.otp
> > qemu-system-riscv64: -device riscv.sifive.u.otp: Parameter 'driver'
> expects pluggable device type
>
> You don't need the -device, -drive should be enough (and then the OTP
> device needs to be re-written to support it).
>
> Alistair
>
> >
> > Then I dump "info qtree". The device, "riscv.sifive.u.otp", belongs to
> 'System' bus. (dump list by 'info qdm') and all devices on 'System' bus
> seem not available with "-device". Any suggestions for specifying the
> device?
> >
> > Thanks,
> > - Green
> >
> > On Tue, Aug 11, 2020 at 6:24 AM Alistair Francis <alistair23@gmail.com>
> wrote:
> >>
> >> ,()On Thu, Jul 30, 2020 at 7:49 PM Green Wan <green.wan@sifive.com>
> wrote:
> >> >
> >> > Add a file-backed implementation for OTP of sifive_u machine. The
> >> > machine property for file-backed is disabled in default. Do file
> >> > open, mmap and close for every OTP read/write in case keep the
> >> > update-to-date snapshot of OTP.
> >>
> >> I don't think this is the correct way to write to the file.
> >>
> >> QEMU has backends that should do this for you. For example QEMU
> >> includes the -blockdev/-driver or -mtdblock command line arguments.
> >>
> >> This implementation should look more like an SD card in terms of
> >> interface. You will probably want to call drive_get_next() (probably
> >> with IF_MTD, but that's up to you).
> >>
> >> The hw/arm/xlnx-zcu102.c file has a good example of attaching an SD
> >> card by setting the drive property.
> >>
> >> Alistair
> >>
> >> >
> >> > Signed-off-by: Green Wan <green.wan@sifive.com>
> >> > ---
> >> > hw/riscv/sifive_u.c | 26 +++++++++++
> >> > hw/riscv/sifive_u_otp.c | 83
> +++++++++++++++++++++++++++++++++
> >> > include/hw/riscv/sifive_u.h | 2 +
> >> > include/hw/riscv/sifive_u_otp.h | 1 +
> >> > 4 files changed, 112 insertions(+)
> >> >
> >> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> >> > index e5682c38a9..c818496918 100644
> >> > --- a/hw/riscv/sifive_u.c
> >> > +++ b/hw/riscv/sifive_u.c
> >> > @@ -87,6 +87,7 @@ static const struct MemmapEntry {
> >> > };
> >> >
> >> > #define OTP_SERIAL 1
> >> > +#define OTP_FILE "NULL"
> >> > #define GEM_REVISION 0x10070109
> >> >
> >> > static void create_fdt(SiFiveUState *s, const struct MemmapEntry
> *memmap,
> >> > @@ -387,6 +388,8 @@ static void sifive_u_machine_init(MachineState
> *machine)
> >> > object_initialize_child(OBJECT(machine), "soc", &s->soc,
> TYPE_RISCV_U_SOC);
> >> > object_property_set_uint(OBJECT(&s->soc), "serial", s->serial,
> >> > &error_abort);
> >> > + object_property_set_str(OBJECT(&s->soc), "otp-file", s->otp_file,
> >> > + &error_abort);
> >> > qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
> >> >
> >> > /* register RAM */
> >> > @@ -526,6 +529,21 @@ static void
> sifive_u_machine_set_uint32_prop(Object *obj, Visitor *v,
> >> > visit_type_uint32(v, name, (uint32_t *)opaque, errp);
> >> > }
> >> >
> >> > +static void sifive_u_machine_get_str_prop(Object *obj, Visitor *v,
> >> > + const char *name, void
> *opaque,
> >> > + Error **errp)
> >> > +{
> >> > + visit_type_str(v, name, (char **)opaque, errp);
> >> > +}
> >> > +
> >> > +static void sifive_u_machine_set_str_prop(Object *obj, Visitor *v,
> >> > + const char *name, void
> *opaque,
> >> > + Error **errp)
> >> > +{
> >> > + visit_type_str(v, name, (char **)opaque, errp);
> >> > +}
> >> > +
> >> > +
> >> > static void sifive_u_machine_instance_init(Object *obj)
> >> > {
> >> > SiFiveUState *s = RISCV_U_MACHINE(obj);
> >> > @@ -551,6 +569,12 @@ static void
> sifive_u_machine_instance_init(Object *obj)
> >> > sifive_u_machine_get_uint32_prop,
> >> > sifive_u_machine_set_uint32_prop, NULL,
> &s->serial);
> >> > object_property_set_description(obj, "serial", "Board serial
> number");
> >> > +
> >> > + s->otp_file = (char *)OTP_FILE;
> >> > + object_property_add(obj, "otp-file", "string",
> >> > + sifive_u_machine_get_str_prop,
> >> > + sifive_u_machine_set_str_prop, NULL,
> &s->otp_file);
> >> > + object_property_set_description(obj, "otp-file", "file-backed
> otp file");
> >> > }
> >> >
> >> > static void sifive_u_machine_class_init(ObjectClass *oc, void *data)
> >> > @@ -709,6 +733,7 @@ static void sifive_u_soc_realize(DeviceState
> *dev, Error **errp)
> >> > }
> >> >
> >> > qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
> >> > + qdev_prop_set_string(DEVICE(&s->otp), "otp-file", s->otp_file);
> >> > if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
> >> > return;
> >> > }
> >> > @@ -737,6 +762,7 @@ static void sifive_u_soc_realize(DeviceState
> *dev, Error **errp)
> >> >
> >> > static Property sifive_u_soc_props[] = {
> >> > DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial,
> OTP_SERIAL),
> >> > + DEFINE_PROP_STRING("otp-file", SiFiveUSoCState, otp_file),
> >> > DEFINE_PROP_END_OF_LIST()
> >> > };
> >> >
> >> > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
> >> > index f6ecbaa2ca..e359f30fdb 100644
> >> > --- a/hw/riscv/sifive_u_otp.c
> >> > +++ b/hw/riscv/sifive_u_otp.c
> >> > @@ -24,6 +24,62 @@
> >> > #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 int32_t sifive_u_otp_backed_open(const char *filename,
> int32_t *fd,
> >> > + uint32_t **map)
> >> > +{
> >> > + /* open and mmap OTP image file */
> >> > + if (filename && strcmp(filename, "NULL") != 0) {
> >> > + *fd = open(filename, O_RDWR);
> >> > +
> >> > + if (*fd < 0) {
> >> > + qemu_log_mask(LOG_TRACE,
> >> > + TRACE_PREFIX "Warning: can't open otp
> file<%s>\n",
> >> > + filename);
> >> > + return -1;
> >> > + } else {
> >> > +
> >> > + *map = (unsigned int *)mmap(0,
> >> > + SIFIVE_FU540_OTP_SIZE,
> >> > + PROT_READ | PROT_WRITE |
> PROT_EXEC,
> >> > + MAP_FILE | MAP_SHARED,
> >> > + *fd,
> >> > + 0);
> >> > +
> >> > + if (*map == MAP_FAILED) {
> >> > + qemu_log_mask(LOG_TRACE,
> >> > + TRACE_PREFIX "Warning: can't mmap otp
> file<%s>\n",
> >> > + filename);
> >> > + close(*fd);
> >> > + return -2;
> >> > + }
> >> > + }
> >> > + } else {
> >> > + /* filename is 'NULL' */
> >> > + return -3;
> >> > + }
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static int32_t sifive_u_otp_backed_close(int fd, unsigned int *map)
> >> > +{
> >> > + munmap(map, SIFIVE_FU540_OTP_SIZE);
> >> > + close(fd);
> >> > +
> >> > + return 0;
> >> > +}
> >> >
> >> > static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr,
> unsigned int size)
> >> > {
> >> > @@ -46,6 +102,20 @@ 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)) {
> >> > +
> >> > + int32_t fd;
> >> > + uint32_t *map;
> >> > + uint64_t val;
> >> > +
> >> > + /* open and mmap OTP image file */
> >> > + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd,
> &map)) {
> >> > + val = (uint64_t)(map[s->pa]);
> >> > +
> >> > + /* close and unmmap */
> >> > + sifive_u_otp_backed_close(fd, map);
> >> > + return val;
> >> > + }
> >> > +
> >> > return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
> >> > } else {
> >> > return 0xff;
> >> > @@ -78,6 +148,8 @@ static void sifive_u_otp_write(void *opaque,
> hwaddr addr,
> >> > {
> >> > SiFiveUOTPState *s = opaque;
> >> > uint32_t val32 = (uint32_t)val64;
> >> > + int32_t fd;
> >> > + uint32_t *map;
> >> >
> >> > switch (addr) {
> >> > case SIFIVE_U_OTP_PA:
> >> > @@ -123,6 +195,16 @@ static void sifive_u_otp_write(void *opaque,
> hwaddr addr,
> >> > s->ptrim = val32;
> >> > break;
> >> > case SIFIVE_U_OTP_PWE:
> >> > + /* open and mmap OTP image file */
> >> > + if (0 == sifive_u_otp_backed_open(s->otp_file, &fd, &map)) {
> >> > + /* store value */
> >> > + map[s->pa] &= ~(s->pdin << s->paio);
> >> > + map[s->pa] |= (s->pdin << s->paio);
> >> > +
> >> > + /* close and unmmap */
> >> > + sifive_u_otp_backed_close(fd, map);
> >> > + }
> >> > +
> >> > s->pwe = val32;
> >> > break;
> >> > default:
> >> > @@ -143,6 +225,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(),
> >> > };
> >> >
> >> > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> >> > index aba4d0181f..966723da1d 100644
> >> > --- a/include/hw/riscv/sifive_u.h
> >> > +++ b/include/hw/riscv/sifive_u.h
> >> > @@ -46,6 +46,7 @@ typedef struct SiFiveUSoCState {
> >> > CadenceGEMState gem;
> >> >
> >> > uint32_t serial;
> >> > + char *otp_file;
> >> > } SiFiveUSoCState;
> >> >
> >> > #define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u")
> >> > @@ -65,6 +66,7 @@ typedef struct SiFiveUState {
> >> > bool start_in_flash;
> >> > uint32_t msel;
> >> > uint32_t serial;
> >> > + char *otp_file;
> >> > } SiFiveUState;
> >> >
> >> > enum {
> >> > diff --git a/include/hw/riscv/sifive_u_otp.h
> b/include/hw/riscv/sifive_u_otp.h
> >> > index 639297564a..f3d71ce82d 100644
> >> > --- a/include/hw/riscv/sifive_u_otp.h
> >> > +++ b/include/hw/riscv/sifive_u_otp.h
> >> > @@ -75,6 +75,7 @@ typedef struct SiFiveUOTPState {
> >> > uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
> >> > /* config */
> >> > uint32_t serial;
> >> > + char *otp_file;
> >> > } SiFiveUOTPState;
> >> >
> >> > #endif /* HW_SIFIVE_U_OTP_H */
> >> > --
> >> > 2.17.1
> >> >
> >> >
>
[-- Attachment #2: Type: text/html, Size: 16815 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-18 17:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31 2:47 [RFC PATCH v2 0/2] Add write-once and file-backed features to OTP Green Wan
2020-07-31 2:47 ` [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP Green Wan
2020-08-10 22:13 ` Alistair Francis
2020-08-13 4:12 ` Green Wan
2020-08-13 21:24 ` Alistair Francis
2020-08-18 17:12 ` Green Wan
2020-07-31 2:47 ` [RFC PATCH v2 2/2] hw/riscv: sifive_u: Add write-once protection Green Wan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).