Re: [PATCH v6 30/30] hw/timer/sh_timer: Remove use of hw_error

[BALATON Zoltan]

On Sat, 30 Oct 2021, Philippe Mathieu-Daudé wrote:
> On 10/29/21 23:02, BALATON Zoltan wrote:
> > The hw_error function calls abort and is not meant to be used by
> > devices. Use qemu_log_mask instead to log and ignore invalid accesses.
> > Also fix format strings to allow dropping type casts of hwaddr and use
> > __func__ instead of hard coding function name in the message which
> > were wrong in two cases.
> >
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > ---
> >  hw/timer/sh_timer.c | 40 +++++++++++++++++++++++++---------------
> >  1 file changed, 25 insertions(+), 15 deletions(-)
> >
> > diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
> > index a6445092e4..8a586f2c4a 100644
> > --- a/hw/timer/sh_timer.c
> > +++ b/hw/timer/sh_timer.c
> > @@ -10,7 +10,7 @@
> >
> >  #include "qemu/osdep.h"
> >  #include "exec/memory.h"
> > -#include "hw/hw.h"
> > +#include "qemu/log.h"
> >  #include "hw/irq.h"
> >  #include "hw/sh4/sh.h"
> >  #include "hw/timer/tmu012.h"
> > @@ -75,11 +75,10 @@ static uint32_t sh_timer_read(void *opaque, hwaddr offset)
> >          if (s->feat & TIMER_FEAT_CAPT) {
> >              return s->tcpr;
> >          }
> > -        /* fall through */
> > -    default:
> > -        hw_error("sh_timer_read: Bad offset %x\n", (int)offset);
> > -        return 0;
> >      }
> > +    qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> > +                  __func__, offset);
> > +    return 0;
>
> Note, keeping the default case allow to refactor for single return,
> so it is easier to add trace event.
>
> Anyhow,
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

It's easy enough to change back if adding a trace point in the future so 
for now I'd leave it at the end outside the if as in this patch. This 
could be revisited when QOM-ifying it in the future, don't think it's a 
big deal.

Regards,
BALATON Zoltan