Re: [PULL 12/12] hw/misc/sifive_u_otp: Add backend drive support

[Peter Maydell]

On Fri, 23 Oct 2020 at 16:27, Alistair Francis <alistair.francis@wdc.com> wrote:
>
> From: Green Wan <green.wan@sifive.com>
>
> Add '-drive' support to OTP device. Allow users to assign a raw file
> as OTP image.

Hi; Coverity reports some issues with this code (CID 1435959,
CID 1435960, CID 1435961). They're all basically the same thing:
in read, write and reset functions this code calls blk_pread()
or blk_pwrite() but doesn't check the return value, so if the
underlying file operation fails then the guest will be
returned garbage data or have its write thrown away without
either the guest or the user being warned about that.

> @@ -54,6 +57,16 @@ 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)) {
> +
> +            /* read from backend */
> +            if (s->blk) {
> +                int32_t buf;
> +
> +                blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf,
> +                          SIFIVE_U_OTP_FUSE_WORD);
> +                return buf;
> +            }
> +
>              return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
>          } else {
>              return 0xff;
> @@ -145,6 +158,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
>              /* write bit data */
>              SET_FUSEARRAY_BIT(s->fuse, s->pa, s->paio, s->pdin);
>
> +            /* write to backend */
> +            if (s->blk) {
> +                blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD,
> +                           &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD, 0);
> +            }
> +
>              /* update written bit */
>              SET_FUSEARRAY_BIT(s->fuse_wo, s->pa, s->paio, WRITTEN_BIT_ON);
>          }
> @@ -168,16 +187,48 @@ static const MemoryRegionOps sifive_u_otp_ops = {

>  static void sifive_u_otp_reset(DeviceState *dev)
> @@ -191,6 +242,20 @@ static void sifive_u_otp_reset(DeviceState *dev)
>      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
>      s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
>
> +    if (s->blk) {
> +        /* Put serial number to backend as well*/
> +        uint32_t serial_data;
> +        int index = SIFIVE_U_OTP_SERIAL_ADDR;
> +
> +        serial_data = s->serial;
> +        blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +
> +        serial_data = ~(s->serial);
> +        blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> +                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +    }
> +
>      /* Initialize write-once map */
>      memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));
>  }
> --
> 2.28.0

thanks
-- PMM