[PATCH 0/3] hw/nvram: Fix Memory Leaks in Xilinx eFUSE

[PATCH 0/3] hw/nvram: Fix Memory Leaks in Xilinx eFUSE

[Tong Ho]

This series fixes memory leaks in Xilinx eFUSE devices for
the Versal and ZynqMP product families.

The leaks result from failing to free memory allocated
by object_get_canonical_path().

Tong Ho (3):
  hw/nvram: Fix Memory Leak in Xilinx eFuse QOM
  hw/nvram: Fix Memory Leak in Xilinx Versal eFuse device
  hw/nvram: Fix Memory Leak in Xilinx ZynqMP eFuse device

 hw/nvram/xlnx-efuse.c             |  9 ++++++---
 hw/nvram/xlnx-versal-efuse-ctrl.c | 20 +++++++++++++++-----
 hw/nvram/xlnx-zynqmp-efuse.c      | 18 ++++++++++++------
 3 files changed, 33 insertions(+), 14 deletions(-)

-- 
2.25.1

[PATCH 1/3] hw/nvram: Fix Memory Leak in Xilinx eFuse QOM

[Tong Ho]

Signed-off-by: Tong Ho <tong.ho@xilinx.com>
---
 hw/nvram/xlnx-efuse.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
index ee1caab54c..a0fd77b586 100644
--- a/hw/nvram/xlnx-efuse.c
+++ b/hw/nvram/xlnx-efuse.c
@@ -144,10 +144,11 @@ static bool efuse_ro_bits_find(XlnxEFuse *s, uint32_t k)
 bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
 {
     if (efuse_ro_bits_find(s, bit)) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         qemu_log_mask(LOG_GUEST_ERROR, "%s: WARN: "
                       "Ignored setting of readonly efuse bit<%u,%u>!\n",
-                      object_get_canonical_path(OBJECT(s)),
-                      (bit / 32), (bit % 32));
+                      path, (bit / 32), (bit % 32));
         return false;
     }
 
@@ -202,9 +203,11 @@ static void efuse_realize(DeviceState *dev, Error **errp)
     efuse_ro_bits_sort(s);
 
     if ((s->efuse_size % 32) != 0) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         error_setg(errp,
                    "%s.efuse-size: %u: property value not multiple of 32.",
-                   object_get_canonical_path(OBJECT(dev)), s->efuse_size);
+                   path, s->efuse_size);
         return;
     }
 
-- 
2.25.1

[PATCH 2/3] hw/nvram: Fix Memory Leak in Xilinx Versal eFuse device

[Tong Ho]

Signed-off-by: Tong Ho <tong.ho@xilinx.com>
---
 hw/nvram/xlnx-versal-efuse-ctrl.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/nvram/xlnx-versal-efuse-ctrl.c b/hw/nvram/xlnx-versal-efuse-ctrl.c
index d362376703..b35ba65ab5 100644
--- a/hw/nvram/xlnx-versal-efuse-ctrl.c
+++ b/hw/nvram/xlnx-versal-efuse-ctrl.c
@@ -439,9 +439,11 @@ static void efuse_pgm_addr_postw(RegisterInfo *reg, uint64_t val64)
      *       up to guest to do so (or by reset).
      */
     if (efuse_pgm_locked(s, bit)) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Denied setting of efuse<%u, %u, %u>\n",
-                      object_get_canonical_path(OBJECT(s)),
+                      path,
                       FIELD_EX32(bit, EFUSE_PGM_ADDR, PAGE),
                       FIELD_EX32(bit, EFUSE_PGM_ADDR, ROW),
                       FIELD_EX32(bit, EFUSE_PGM_ADDR, COLUMN));
@@ -478,9 +480,11 @@ static void efuse_rd_addr_postw(RegisterInfo *reg, uint64_t val64)
     s->regs[R_EFUSE_RD_DATA] = xlnx_versal_efuse_read_row(s->efuse,
                                                           bit, &denied);
     if (denied) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Denied reading of efuse<%u, %u>\n",
-                      object_get_canonical_path(OBJECT(s)),
+                      path,
                       FIELD_EX32(bit, EFUSE_RD_ADDR, PAGE),
                       FIELD_EX32(bit, EFUSE_RD_ADDR, ROW));
 
@@ -625,9 +629,11 @@ static void efuse_ctrl_reg_write(void *opaque, hwaddr addr,
     s = XLNX_VERSAL_EFUSE_CTRL(dev);
 
     if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s[reg_0x%02lx]: Attempt to write locked register.\n",
-                      object_get_canonical_path(OBJECT(s)), (long)addr);
+                      path, (long)addr);
     } else {
         register_write_memory(opaque, addr, data, size);
     }
@@ -681,16 +687,20 @@ static void efuse_ctrl_realize(DeviceState *dev, Error **errp)
     const uint32_t lks_sz = sizeof(XlnxEFuseLkSpec) / 2;
 
     if (!s->efuse) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         error_setg(errp, "%s.efuse: link property not connected to XLNX-EFUSE",
-                   object_get_canonical_path(OBJECT(dev)));
+                   path);
         return;
     }
 
     /* Sort property-defined pgm-locks for bsearch lookup */
     if ((s->extra_pg0_lock_n16 % lks_sz) != 0) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         error_setg(errp,
                    "%s.pg0-lock: array property item-count not multiple of %u",
-                   object_get_canonical_path(OBJECT(dev)), lks_sz);
+                   path, lks_sz);
         return;
     }
 
-- 
2.25.1

[PATCH 3/3] hw/nvram: Fix Memory Leak in Xilinx ZynqMP eFuse device

[Tong Ho]

Signed-off-by: Tong Ho <tong.ho@xilinx.com>
---
 hw/nvram/xlnx-zynqmp-efuse.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/nvram/xlnx-zynqmp-efuse.c b/hw/nvram/xlnx-zynqmp-efuse.c
index 1f87dbf988..228ba0bbfa 100644
--- a/hw/nvram/xlnx-zynqmp-efuse.c
+++ b/hw/nvram/xlnx-zynqmp-efuse.c
@@ -434,11 +434,12 @@ static void zynqmp_efuse_pgm_addr_postw(RegisterInfo *reg, uint64_t val64)
     if (!errmsg) {
         ARRAY_FIELD_DP32(s->regs, EFUSE_ISR, PGM_ERROR, 0);
     } else {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         ARRAY_FIELD_DP32(s->regs, EFUSE_ISR, PGM_ERROR, 1);
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s - eFuse write error: %s; addr=0x%x\n",
-                      object_get_canonical_path(OBJECT(s)),
-                      errmsg, (unsigned)val64);
+                      path, errmsg, (unsigned)val64);
     }
 
     ARRAY_FIELD_DP32(s->regs, EFUSE_ISR, PGM_DONE, 1);
@@ -448,6 +449,7 @@ static void zynqmp_efuse_pgm_addr_postw(RegisterInfo *reg, uint64_t val64)
 static void zynqmp_efuse_rd_addr_postw(RegisterInfo *reg, uint64_t val64)
 {
     XlnxZynqMPEFuse *s = XLNX_ZYNQMP_EFUSE(reg->opaque);
+    g_autofree char *path = NULL;
 
     /*
      * Grant reads only to allowed bits; reference sources:
@@ -538,10 +540,10 @@ static void zynqmp_efuse_rd_addr_postw(RegisterInfo *reg, uint64_t val64)
     return;
 
  denied:
+    path = object_get_canonical_path(OBJECT(s));
     qemu_log_mask(LOG_GUEST_ERROR,
                   "%s: Denied efuse read from array %u, row %u\n",
-                  object_get_canonical_path(OBJECT(s)),
-                  efuse_ary, efuse_row);
+                  path, efuse_ary, efuse_row);
 
     s->regs[R_EFUSE_RD_DATA] = 0;
 
@@ -731,9 +733,11 @@ static void zynqmp_efuse_reg_write(void *opaque, hwaddr addr,
     s = XLNX_ZYNQMP_EFUSE(dev);
 
     if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s[reg_0x%02lx]: Attempt to write locked register.\n",
-                      object_get_canonical_path(OBJECT(s)), (long)addr);
+                      path, (long)addr);
     } else {
         register_write_memory(opaque, addr, data, size);
     }
@@ -784,8 +788,10 @@ static void zynqmp_efuse_realize(DeviceState *dev, Error **errp)
     XlnxZynqMPEFuse *s = XLNX_ZYNQMP_EFUSE(dev);
 
     if (!s->efuse) {
+        g_autofree char *path = object_get_canonical_path(OBJECT(s));
+
         error_setg(errp, "%s.efuse: link property not connected to XLNX-EFUSE",
-                   object_get_canonical_path(OBJECT(dev)));
+                   path);
         return;
     }
 
-- 
2.25.1

Re: [PATCH 0/3] hw/nvram: Fix Memory Leaks in Xilinx eFUSE

[Philippe Mathieu-Daudé]

Cc'ing qemu-trivial@

On 10/15/21 22:35, Tong Ho wrote:
> This series fixes memory leaks in Xilinx eFUSE devices for
> the Versal and ZynqMP product families.
> 
> The leaks result from failing to free memory allocated
> by object_get_canonical_path().
> 
> Tong Ho (3):
>   hw/nvram: Fix Memory Leak in Xilinx eFuse QOM
>   hw/nvram: Fix Memory Leak in Xilinx Versal eFuse device
>   hw/nvram: Fix Memory Leak in Xilinx ZynqMP eFuse device
> 
>  hw/nvram/xlnx-efuse.c             |  9 ++++++---
>  hw/nvram/xlnx-versal-efuse-ctrl.c | 20 +++++++++++++++-----
>  hw/nvram/xlnx-zynqmp-efuse.c      | 18 ++++++++++++------
>  3 files changed, 33 insertions(+), 14 deletions(-)
> 

Series:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 1/3] hw/nvram: Fix Memory Leak in Xilinx eFuse QOM

[Francisco Iglesias]

On [2021 Oct 15] Fri 13:35:30, Tong Ho wrote:
> Signed-off-by: Tong Ho <tong.ho@xilinx.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/nvram/xlnx-efuse.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
> index ee1caab54c..a0fd77b586 100644
> --- a/hw/nvram/xlnx-efuse.c
> +++ b/hw/nvram/xlnx-efuse.c
> @@ -144,10 +144,11 @@ static bool efuse_ro_bits_find(XlnxEFuse *s, uint32_t k)
>  bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit)
>  {
>      if (efuse_ro_bits_find(s, bit)) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: WARN: "
>                        "Ignored setting of readonly efuse bit<%u,%u>!\n",
> -                      object_get_canonical_path(OBJECT(s)),
> -                      (bit / 32), (bit % 32));
> +                      path, (bit / 32), (bit % 32));
>          return false;
>      }
>  
> @@ -202,9 +203,11 @@ static void efuse_realize(DeviceState *dev, Error **errp)
>      efuse_ro_bits_sort(s);
>  
>      if ((s->efuse_size % 32) != 0) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          error_setg(errp,
>                     "%s.efuse-size: %u: property value not multiple of 32.",
> -                   object_get_canonical_path(OBJECT(dev)), s->efuse_size);
> +                   path, s->efuse_size);
>          return;
>      }
>  
> -- 
> 2.25.1
> 
> 

Re: [PATCH 2/3] hw/nvram: Fix Memory Leak in Xilinx Versal eFuse device

[Francisco Iglesias]

On [2021 Oct 15] Fri 13:35:31, Tong Ho wrote:
> Signed-off-by: Tong Ho <tong.ho@xilinx.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/nvram/xlnx-versal-efuse-ctrl.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/nvram/xlnx-versal-efuse-ctrl.c b/hw/nvram/xlnx-versal-efuse-ctrl.c
> index d362376703..b35ba65ab5 100644
> --- a/hw/nvram/xlnx-versal-efuse-ctrl.c
> +++ b/hw/nvram/xlnx-versal-efuse-ctrl.c
> @@ -439,9 +439,11 @@ static void efuse_pgm_addr_postw(RegisterInfo *reg, uint64_t val64)
>       *       up to guest to do so (or by reset).
>       */
>      if (efuse_pgm_locked(s, bit)) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Denied setting of efuse<%u, %u, %u>\n",
> -                      object_get_canonical_path(OBJECT(s)),
> +                      path,
>                        FIELD_EX32(bit, EFUSE_PGM_ADDR, PAGE),
>                        FIELD_EX32(bit, EFUSE_PGM_ADDR, ROW),
>                        FIELD_EX32(bit, EFUSE_PGM_ADDR, COLUMN));
> @@ -478,9 +480,11 @@ static void efuse_rd_addr_postw(RegisterInfo *reg, uint64_t val64)
>      s->regs[R_EFUSE_RD_DATA] = xlnx_versal_efuse_read_row(s->efuse,
>                                                            bit, &denied);
>      if (denied) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Denied reading of efuse<%u, %u>\n",
> -                      object_get_canonical_path(OBJECT(s)),
> +                      path,
>                        FIELD_EX32(bit, EFUSE_RD_ADDR, PAGE),
>                        FIELD_EX32(bit, EFUSE_RD_ADDR, ROW));
>  
> @@ -625,9 +629,11 @@ static void efuse_ctrl_reg_write(void *opaque, hwaddr addr,
>      s = XLNX_VERSAL_EFUSE_CTRL(dev);
>  
>      if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s[reg_0x%02lx]: Attempt to write locked register.\n",
> -                      object_get_canonical_path(OBJECT(s)), (long)addr);
> +                      path, (long)addr);
>      } else {
>          register_write_memory(opaque, addr, data, size);
>      }
> @@ -681,16 +687,20 @@ static void efuse_ctrl_realize(DeviceState *dev, Error **errp)
>      const uint32_t lks_sz = sizeof(XlnxEFuseLkSpec) / 2;
>  
>      if (!s->efuse) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          error_setg(errp, "%s.efuse: link property not connected to XLNX-EFUSE",
> -                   object_get_canonical_path(OBJECT(dev)));
> +                   path);
>          return;
>      }
>  
>      /* Sort property-defined pgm-locks for bsearch lookup */
>      if ((s->extra_pg0_lock_n16 % lks_sz) != 0) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          error_setg(errp,
>                     "%s.pg0-lock: array property item-count not multiple of %u",
> -                   object_get_canonical_path(OBJECT(dev)), lks_sz);
> +                   path, lks_sz);
>          return;
>      }
>  
> -- 
> 2.25.1
> 
> 

Re: [PATCH 3/3] hw/nvram: Fix Memory Leak in Xilinx ZynqMP eFuse device

[Francisco Iglesias]

On [2021 Oct 15] Fri 13:35:32, Tong Ho wrote:
> Signed-off-by: Tong Ho <tong.ho@xilinx.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/nvram/xlnx-zynqmp-efuse.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/nvram/xlnx-zynqmp-efuse.c b/hw/nvram/xlnx-zynqmp-efuse.c
> index 1f87dbf988..228ba0bbfa 100644
> --- a/hw/nvram/xlnx-zynqmp-efuse.c
> +++ b/hw/nvram/xlnx-zynqmp-efuse.c
> @@ -434,11 +434,12 @@ static void zynqmp_efuse_pgm_addr_postw(RegisterInfo *reg, uint64_t val64)
>      if (!errmsg) {
>          ARRAY_FIELD_DP32(s->regs, EFUSE_ISR, PGM_ERROR, 0);
>      } else {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          ARRAY_FIELD_DP32(s->regs, EFUSE_ISR, PGM_ERROR, 1);
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s - eFuse write error: %s; addr=0x%x\n",
> -                      object_get_canonical_path(OBJECT(s)),
> -                      errmsg, (unsigned)val64);
> +                      path, errmsg, (unsigned)val64);
>      }
>  
>      ARRAY_FIELD_DP32(s->regs, EFUSE_ISR, PGM_DONE, 1);
> @@ -448,6 +449,7 @@ static void zynqmp_efuse_pgm_addr_postw(RegisterInfo *reg, uint64_t val64)
>  static void zynqmp_efuse_rd_addr_postw(RegisterInfo *reg, uint64_t val64)
>  {
>      XlnxZynqMPEFuse *s = XLNX_ZYNQMP_EFUSE(reg->opaque);
> +    g_autofree char *path = NULL;
>  
>      /*
>       * Grant reads only to allowed bits; reference sources:
> @@ -538,10 +540,10 @@ static void zynqmp_efuse_rd_addr_postw(RegisterInfo *reg, uint64_t val64)
>      return;
>  
>   denied:
> +    path = object_get_canonical_path(OBJECT(s));
>      qemu_log_mask(LOG_GUEST_ERROR,
>                    "%s: Denied efuse read from array %u, row %u\n",
> -                  object_get_canonical_path(OBJECT(s)),
> -                  efuse_ary, efuse_row);
> +                  path, efuse_ary, efuse_row);
>  
>      s->regs[R_EFUSE_RD_DATA] = 0;
>  
> @@ -731,9 +733,11 @@ static void zynqmp_efuse_reg_write(void *opaque, hwaddr addr,
>      s = XLNX_ZYNQMP_EFUSE(dev);
>  
>      if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s[reg_0x%02lx]: Attempt to write locked register.\n",
> -                      object_get_canonical_path(OBJECT(s)), (long)addr);
> +                      path, (long)addr);
>      } else {
>          register_write_memory(opaque, addr, data, size);
>      }
> @@ -784,8 +788,10 @@ static void zynqmp_efuse_realize(DeviceState *dev, Error **errp)
>      XlnxZynqMPEFuse *s = XLNX_ZYNQMP_EFUSE(dev);
>  
>      if (!s->efuse) {
> +        g_autofree char *path = object_get_canonical_path(OBJECT(s));
> +
>          error_setg(errp, "%s.efuse: link property not connected to XLNX-EFUSE",
> -                   object_get_canonical_path(OBJECT(dev)));
> +                   path);
>          return;
>      }
>  
> -- 
> 2.25.1
> 
> 

Re: [PATCH 0/3] hw/nvram: Fix Memory Leaks in Xilinx eFUSE

[Edgar E. Iglesias]

On Mon, Oct 18, 2021 at 12:31:44PM +0200, Philippe Mathieu-Daudé wrote:
> Cc'ing qemu-trivial@
> 
> On 10/15/21 22:35, Tong Ho wrote:
> > This series fixes memory leaks in Xilinx eFUSE devices for
> > the Versal and ZynqMP product families.
> > 
> > The leaks result from failing to free memory allocated
> > by object_get_canonical_path().
> > 
> > Tong Ho (3):
> >   hw/nvram: Fix Memory Leak in Xilinx eFuse QOM
> >   hw/nvram: Fix Memory Leak in Xilinx Versal eFuse device
> >   hw/nvram: Fix Memory Leak in Xilinx ZynqMP eFuse device
> > 
> >  hw/nvram/xlnx-efuse.c             |  9 ++++++---
> >  hw/nvram/xlnx-versal-efuse-ctrl.c | 20 +++++++++++++++-----
> >  hw/nvram/xlnx-zynqmp-efuse.c      | 18 ++++++++++++------
> >  3 files changed, 33 insertions(+), 14 deletions(-)
> > 
> 
> Series:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> 

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Re: [PATCH 0/3] hw/nvram: Fix Memory Leaks in Xilinx eFUSE

[Laurent Vivier]

Le 15/10/2021 à 22:35, Tong Ho a écrit :
> This series fixes memory leaks in Xilinx eFUSE devices for
> the Versal and ZynqMP product families.
> 
> The leaks result from failing to free memory allocated
> by object_get_canonical_path().
> 
> Tong Ho (3):
>   hw/nvram: Fix Memory Leak in Xilinx eFuse QOM
>   hw/nvram: Fix Memory Leak in Xilinx Versal eFuse device
>   hw/nvram: Fix Memory Leak in Xilinx ZynqMP eFuse device
> 
>  hw/nvram/xlnx-efuse.c             |  9 ++++++---
>  hw/nvram/xlnx-versal-efuse-ctrl.c | 20 +++++++++++++++-----
>  hw/nvram/xlnx-zynqmp-efuse.c      | 18 ++++++++++++------
>  3 files changed, 33 insertions(+), 14 deletions(-)
> 

Applied to my trivial-patches branch.

Thanks,
Laurent