Re: [RFC PATCH v2 1/2] hw/riscv: sifive

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
>
>

From:	Green Wan
Subject:	Re: [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP.
Date:	Thu, 13 Aug 2020 12:12:20 +0800

Re: [RFC PATCH v2 1/2] hw/riscv: sifive_u: Add file-backed OTP.