qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


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

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]