diff mbox series

[v4,3/3] xilink-zynq-devcfg: Avoid disabling devcfg memory region during initialization

Message ID bcdacdcbf7b26192aebe4723c57bd87ef986defa.1728299530.git.chao.liu@yeah.net (mailing list archive)
State New, archived
Headers show
Series Drop ignore_memory_transaction_failures for xilink_zynq | expand

Commit Message

Chao Liu Oct. 7, 2024, 11:24 a.m. UTC
During the initialization phase, we've encountered an issue where the
UNLOCK register is inadvertently cleared. This results in devcfg MR being
disabled, which in turn leads to unexpected memory access exceptions when
attempting subsequent accesses to the devcfg register. This behavior is not
consistent with the hardware specifications.

This bug was not found earlier because the ignore_memory_transaction_failures
flag was enabled, which ignored exceptions from devcfg devices
when access was disabled.

Signed-off-by: Chao Liu <chao.liu@yeah.net>
---
 hw/dma/xlnx-zynq-devcfg.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Peter Maydell Oct. 14, 2024, 3:15 p.m. UTC | #1
On Mon, 7 Oct 2024 at 12:25, Chao Liu <chao.liu@yeah.net> wrote:
>
> During the initialization phase, we've encountered an issue where the
> UNLOCK register is inadvertently cleared. This results in devcfg MR being
> disabled, which in turn leads to unexpected memory access exceptions when
> attempting subsequent accesses to the devcfg register. This behavior is not
> consistent with the hardware specifications.
>
> This bug was not found earlier because the ignore_memory_transaction_failures
> flag was enabled, which ignored exceptions from devcfg devices
> when access was disabled.
>
> Signed-off-by: Chao Liu <chao.liu@yeah.net>
> ---
>  hw/dma/xlnx-zynq-devcfg.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
> index e5eff9abc0..af8cc72471 100644
> --- a/hw/dma/xlnx-zynq-devcfg.c
> +++ b/hw/dma/xlnx-zynq-devcfg.c
> @@ -144,7 +144,12 @@ static void xlnx_zynq_devcfg_reset(DeviceState *dev)
>      int i;
>
>      for (i = 0; i < XLNX_ZYNQ_DEVCFG_R_MAX; ++i) {
> -        register_reset(&s->regs_info[i]);
> +        if (s->regs_info[i].access) {
> +            if (s->regs_info[i].access->addr == A_UNLOCK) {
> +                continue;
> +            }
> +            register_reset(&s->regs_info[i]);
> +        }
>      }

This looks strange. Reset should reset things. If the
UNLOCK register's reset behaviour is wrong, then shouldn't
we fix it, rather than explicitly skipping resetting it ?

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
index e5eff9abc0..af8cc72471 100644
--- a/hw/dma/xlnx-zynq-devcfg.c
+++ b/hw/dma/xlnx-zynq-devcfg.c
@@ -144,7 +144,12 @@  static void xlnx_zynq_devcfg_reset(DeviceState *dev)
     int i;
 
     for (i = 0; i < XLNX_ZYNQ_DEVCFG_R_MAX; ++i) {
-        register_reset(&s->regs_info[i]);
+        if (s->regs_info[i].access) {
+            if (s->regs_info[i].access->addr == A_UNLOCK) {
+                continue;
+            }
+            register_reset(&s->regs_info[i]);
+        }
     }
 }