Re: [PATCH] hw/misc/sifive_u_otp: handling the fails of blk_pread and blk_pwrite

[Alistair Francis]

On Fri, Jan 15, 2021 at 3:50 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Ping! This patch was trying to fix a Coverity issue (CID 1435959,
> 1435960, 1435961) -- is anybody planning to review it?
>
> (I'm not entirely sure 'guest error' is the right warning category,
> but I don't know the specifics of this device.)

Thanks for the ping, this feel through the cracks somehow.

Applied to riscv-to-apply.next

Alistair

>
> thanks
> -- PMM
>
> On Wed, 4 Nov 2020 at 09:29, Green Wan <green.wan@sifive.com> wrote:
> >
> > Fix code coverage issues by checking return value and handling fail case
> > of blk_pread() and blk_pwrite(). Return default value 0xff if read fails.
> >
> > Signed-off-by: Green Wan <green.wan@sifive.com>
> > ---
> >  hw/misc/sifive_u_otp.c | 31 +++++++++++++++++++++++--------
> >  1 file changed, 23 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
> > index 60066375ab..4314727d0d 100644
> > --- a/hw/misc/sifive_u_otp.c
> > +++ b/hw/misc/sifive_u_otp.c
> > @@ -62,8 +62,13 @@ static uint64_t sifive_u_otp_read(void *opaque, hwaddr 
> > addr, unsigned int size)
> >              if (s->blk) {
> >                  int32_t buf;
> >
> > -                blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf,
> > -                          SIFIVE_U_OTP_FUSE_WORD);
> > +                if (blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf,
> > +                              SIFIVE_U_OTP_FUSE_WORD) < 0) {
> > +                    qemu_log_mask(LOG_GUEST_ERROR,
> > +                                  "read error index<%d>\n", s->pa);
> > +                    return 0xff;
> > +                }
> > +
> >                  return buf;
> >              }
> >
> > @@ -160,8 +165,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr 
> > addr,
> >
> >              /* 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);
> > +                if (blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD,
> > +                               &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD,
> > +                               0) < 0) {
> > +                    qemu_log_mask(LOG_GUEST_ERROR,
> > +                                  "write error index<%d>\n", s->pa);
> > +                }
> >              }
> >
> >              /* update written bit */
> > @@ -248,12 +257,18 @@ static void sifive_u_otp_reset(DeviceState *dev)
> >          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);
> > +        if (blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> > +                       &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0) < 0) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                          "write error index<%d>\n", index);
> > +        }
> >
> >          serial_data = ~(s->serial);
> > -        blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> > -                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> > +        if (blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> > +                       &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0) < 0) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                          "write error index<%d>\n", index + 1);
> > +        }
> >      }
> >
> >      /* Initialize write-once map */
> > --
> > 2.17.1