On 7/9/21 7:31 AM, Andrew Jeffery wrote: > While some of the critical fields remain the same, there is variation in > the definition of the control register across the SoC generations. > Reserved regions are adjusted, while in other cases the mutability or > behaviour of fields change. > > Introduce a callback to sanitize the value on writes to ensure model > behaviour reflects the hardware. > > Fixes: 854123bf8d4b ("wdt: Add Aspeed watchdog device model") > Signed-off-by: Andrew Jeffery <and...@aj.id.au>
Reviewed-by: Cédric Le Goater <c...@kaod.org> > --- > hw/watchdog/wdt_aspeed.c | 24 ++++++++++++++++++++++-- > include/hw/watchdog/wdt_aspeed.h | 1 + > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c > index 6352ba1b0e5b..faa3d35fdf21 100644 > --- a/hw/watchdog/wdt_aspeed.c > +++ b/hw/watchdog/wdt_aspeed.c > @@ -118,13 +118,27 @@ static void aspeed_wdt_reload_1mhz(AspeedWDTState *s) > } > } > > +static uint64_t aspeed_2400_sanitize_ctrl(uint64_t data) > +{ > + return data & 0xffff; > +} > + > +static uint64_t aspeed_2500_sanitize_ctrl(uint64_t data) > +{ > + return (data & ~(0xfUL << 8)) | WDT_CTRL_1MHZ_CLK; > +} > + > +static uint64_t aspeed_2600_sanitize_ctrl(uint64_t data) > +{ > + return data & ~(0x7UL << 7); > +} > > static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data, > unsigned size) > { > AspeedWDTState *s = ASPEED_WDT(opaque); > AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(s); > - bool enable = data & WDT_CTRL_ENABLE; > + bool enable; > > offset >>= 2; > > @@ -144,6 +158,8 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, > uint64_t data, > } > break; > case WDT_CTRL: > + data = awc->sanitize_ctrl(data); > + enable = data & WDT_CTRL_ENABLE; > if (enable && !aspeed_wdt_is_enabled(s)) { > s->regs[WDT_CTRL] = data; > awc->wdt_reload(s); > @@ -207,11 +223,12 @@ static const MemoryRegionOps aspeed_wdt_ops = { > static void aspeed_wdt_reset(DeviceState *dev) > { > AspeedWDTState *s = ASPEED_WDT(dev); > + AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(s); > > s->regs[WDT_STATUS] = 0x3EF1480; > s->regs[WDT_RELOAD_VALUE] = 0x03EF1480; > s->regs[WDT_RESTART] = 0; > - s->regs[WDT_CTRL] = 0; > + s->regs[WDT_CTRL] = awc->sanitize_ctrl(0); > s->regs[WDT_RESET_WIDTH] = 0xFF; > > timer_del(s->timer); > @@ -293,6 +310,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass > *klass, void *data) > awc->ext_pulse_width_mask = 0xff; > awc->reset_ctrl_reg = SCU_RESET_CONTROL1; > awc->wdt_reload = aspeed_wdt_reload; > + awc->sanitize_ctrl = aspeed_2400_sanitize_ctrl; > } > > static const TypeInfo aspeed_2400_wdt_info = { > @@ -328,6 +346,7 @@ static void aspeed_2500_wdt_class_init(ObjectClass > *klass, void *data) > awc->reset_ctrl_reg = SCU_RESET_CONTROL1; > awc->reset_pulse = aspeed_2500_wdt_reset_pulse; > awc->wdt_reload = aspeed_wdt_reload_1mhz; > + awc->sanitize_ctrl = aspeed_2500_sanitize_ctrl; > } > > static const TypeInfo aspeed_2500_wdt_info = { > @@ -348,6 +367,7 @@ static void aspeed_2600_wdt_class_init(ObjectClass > *klass, void *data) > awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1; > awc->reset_pulse = aspeed_2500_wdt_reset_pulse; > awc->wdt_reload = aspeed_wdt_reload_1mhz; > + awc->sanitize_ctrl = aspeed_2600_sanitize_ctrl; > } > > static const TypeInfo aspeed_2600_wdt_info = { > diff --git a/include/hw/watchdog/wdt_aspeed.h > b/include/hw/watchdog/wdt_aspeed.h > index 80b03661e303..f945cd6c5833 100644 > --- a/include/hw/watchdog/wdt_aspeed.h > +++ b/include/hw/watchdog/wdt_aspeed.h > @@ -44,6 +44,7 @@ struct AspeedWDTClass { > uint32_t reset_ctrl_reg; > void (*reset_pulse)(AspeedWDTState *s, uint32_t property); > void (*wdt_reload)(AspeedWDTState *s); > + uint64_t (*sanitize_ctrl)(uint64_t data); > }; > > #endif /* WDT_ASPEED_H */ >