Re: [PATCH 04/21] hw/arm/zynq-devcfg: Prevent unintended unlock during initialization

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Tue, Mar 18, 2025 at 02:07:55PM +0100, Corvin Köhne wrote:
> From: YannickV <Y.Vossen@beckhoff.com>
> 
> During the emulation startup, all registers are reset, which triggers the
> `r_unlock_post_write` function with a value of 0. This led to an
> unintended memory access disable, making the devcfg unusable.
> 
> To address this, a property 'is_initialized' is introduced. It is set
> to false during reset and updated to true once the initialization is
> complete. The unlock function is simply ignored while 'is_initialized'
> is false.
> 
> I have no idea how this ever worked. Nevertheless, this restores the
> correct behavior.
> 
> Signed-off-by: Yannick Voßen <y.vossen@beckhoff.com>
> ---
>  hw/dma/xlnx-zynq-devcfg.c         | 6 +++++-
>  include/hw/dma/xlnx-zynq-devcfg.h | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
> index b838c1c0d0..03b5280228 100644
> --- a/hw/dma/xlnx-zynq-devcfg.c
> +++ b/hw/dma/xlnx-zynq-devcfg.c
> @@ -143,9 +143,11 @@ static void xlnx_zynq_devcfg_reset(DeviceState *dev)
>      XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(dev);
>      int i;
>  
> +    s->is_initialized = false;
>      for (i = 0; i < XLNX_ZYNQ_DEVCFG_R_MAX; ++i) {
>          register_reset(&s->regs_info[i]);
>      }
> +    s->is_initialized = true;
>  }
>  
>  static void xlnx_zynq_devcfg_dma_go(XlnxZynqDevcfg *s)
> @@ -221,7 +223,9 @@ static void r_unlock_post_write(RegisterInfo *reg, uint64_t val)
>  {
>      XlnxZynqDevcfg *s = XLNX_ZYNQ_DEVCFG(reg->opaque);
>      const char *device_prefix = object_get_typename(OBJECT(s));
> -
> +    if (!s->is_initialized) {


I think you can use device_is_in_reset(DEVICE(s))
and get rid of is_initialized alltogether?




> +        return;
> +    }
>      if (val == R_UNLOCK_MAGIC) {
>          DB_PRINT("successful unlock\n");
>          s->regs[R_CTRL] |= R_CTRL_PCAP_PR_MASK;
> diff --git a/include/hw/dma/xlnx-zynq-devcfg.h b/include/hw/dma/xlnx-zynq-devcfg.h
> index e4cf085d70..2ab054e598 100644
> --- a/include/hw/dma/xlnx-zynq-devcfg.h
> +++ b/include/hw/dma/xlnx-zynq-devcfg.h
> @@ -55,6 +55,8 @@ struct XlnxZynqDevcfg {
>      XlnxZynqDevcfgDMACmd dma_cmd_fifo[XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN];
>      uint8_t dma_cmd_fifo_num;
>  
> +    bool is_initialized;
> +
>      uint32_t regs[XLNX_ZYNQ_DEVCFG_R_MAX];
>      RegisterInfo regs_info[XLNX_ZYNQ_DEVCFG_R_MAX];
>  };
> -- 
> 2.49.0
>