[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] watchdog: aspeed: Sanitize control register values
From: |
Cédric Le Goater |
Subject: |
Re: [PATCH 1/2] watchdog: aspeed: Sanitize control register values |
Date: |
Mon, 19 Jul 2021 17:53:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
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 <andrew@aj.id.au>
Reviewed-by: Cédric Le Goater <clg@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 */
>