Hi Bernhard,
On Sun, Oct 4, 2026 at 8:29 PM Bernhard Beschow <[email protected]> wrote:
>
> Guests such as u-boot may experience inadvertant reboots when configuring
> the WCR register. Apparently there is a typo in the WCR locked bits
> handling: The locked values for s->wcr must come from itself rather than
> from s->wicr, so fix that.
>
> Fixes: daca13d495a1 ("hw/watchdog: Implement full i.MX watchdog support")
> cc: qemu-stable
> Signed-off-by: Bernhard Beschow <[email protected]>
> ---
> hw/watchdog/wdt_imx2.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/watchdog/wdt_imx2.c b/hw/watchdog/wdt_imx2.c
> index c83d4107c1..3283990bbf 100644
> --- a/hw/watchdog/wdt_imx2.c
> +++ b/hw/watchdog/wdt_imx2.c
> @@ -156,18 +156,18 @@ static void imx2_wdt_write(void *opaque, hwaddr addr,
> case IMX2_WDT_WCR:
> if (s->wcr_locked) {
> value &= ~IMX2_WDT_WCR_LOCK_MASK;
> - value |= (s->wicr & IMX2_WDT_WCR_LOCK_MASK);
> + value |= (s->wcr & IMX2_WDT_WCR_LOCK_MASK);
I see this patch has qemu-stable cc'ed, so it shall be a complete fix
instead of a partial one that relies one another patch (in this case,
patch 03 in this series).
So patch 1 corrects the source register from s->wicr to s->wcr, but
the WDE and WDT branches still use inverted masks:
> s->wcr & ~IMX2_WDT_WCR_WDE
> s->wcr & ~IMX2_WDT_WCR_WDT
These preserve all the other bits rather than the locked bit, which is
apparently wrong.
Patch 3 fixes this as part of removing the redundant lock flags, but
not marked as qemu-stable intentionally. When applying patch 1 & 3
together on QEMU master, this issue is fixed correctly, but not for
stable.
Could you fix the WDE/WDT masks in patch 1, leaving patch 3 to remove
the redundant flags and migration fields? That would make the stable
fixes work correctly without depending on patch 3.
> }
> s->wcr_locked = true;
> if (s->wcr_wde_locked) {
> value &= ~IMX2_WDT_WCR_WDE;
> - value |= (s->wicr & ~IMX2_WDT_WCR_WDE);
> + value |= (s->wcr & ~IMX2_WDT_WCR_WDE);
> } else if (value & IMX2_WDT_WCR_WDE) {
> s->wcr_wde_locked = true;
> }
> if (s->wcr_wdt_locked) {
> value &= ~IMX2_WDT_WCR_WDT;
> - value |= (s->wicr & ~IMX2_WDT_WCR_WDT);
> + value |= (s->wcr & ~IMX2_WDT_WCR_WDT);
> } else if (value & IMX2_WDT_WCR_WDT) {
> s->wcr_wdt_locked = true;
> }
> --
Regards,
Bin
