Re: [PATCH v3 0/6] Register API leaks fixes

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 17/10/25 18:17, Luc Michel wrote:
> v3:
>    - Rebased on master
>    - Fixed compilation issues in intermediate patches [Phil]
>    - Parent the memory region in the REGISTER_ARRAY object to the
>      REGISTER_ARRAY object itself instead of the REGISTER_ARRAY owner.
>      This ensure correct finalizing order and fixes the use-after-free
>      encountered by Phil [Phil]


> Luc Michel (6):
>    hw/core/register: remove the REGISTER device type
>    hw/core/register: add the REGISTER_ARRAY type
>    hw/core/register: remove the calls to `register_finalize_block'
>    hw/core/register: remove the `register_finalize_block' function
>    hw/net/can/xlnx-versal-canfd: refactor the banked registers logic
>    hw/net/can/xlnx-versal-canfd: remove register API usage for banked
>      regs

Thanks, queued squashing on patch #5 ...:

-- >8 --
--- a/hw/net/can/xlnx-versal-canfd.c
+++ b/hw/net/can/xlnx-versal-canfd.c
@@ -1411,18 +1411,17 @@ static uint64_t canfd_srr_pre_write(RegisterInfo 
*reg, uint64_t val64)
  }

  static void filter_reg_write(XlnxVersalCANFDState *s, hwaddr addr,
-                             size_t bank_idx, uint32_t val)
+                             unsigned bank_idx, uint32_t val)
  {
      size_t reg_idx = addr / sizeof(uint32_t);

      if (!(s->regs[R_ACCEPTANCE_FILTER_CONTROL_REGISTER] &
          (1 << bank_idx))) {
          s->regs[reg_idx] = val;
      } else {
          g_autofree char *path = object_get_canonical_path(OBJECT(s));

          qemu_log_mask(LOG_GUEST_ERROR, "%s: Acceptance filter register 
0x%"
-                      HWADDR_PRIx " changed while filter %zu enabled\n",
+                      HWADDR_PRIx " changed while filter %u enabled\n",
                        path, addr, bank_idx + 1);
      }
  }
@@ -1782,16 +1781,19 @@ static void xlnx_versal_canfd_ptimer_cb(void 
*opaque)

  static bool canfd_decode_reg_bank(XlnxVersalCANFDState *s, hwaddr addr,
                                    hwaddr first_reg, hwaddr last_reg,
-                                  size_t num_banks, size_t *idx, size_t 
*offset)
+                                  size_t num_banks, unsigned *idx,
+                                  hwaddr *offset)
  {
      hwaddr base = addr - first_reg;
      hwaddr span = last_reg - first_reg + sizeof(uint32_t);
+    unsigned index = base / span;

-    *idx = base / span;
-
-    if (*idx >= num_banks) {
+    if (index >= num_banks) {
          return false;
      }
+    if (idx) {
+        *idx = index;
+    }

      *offset = base % span;
      *offset += first_reg;
@@ -1807,7 +1809,7 @@ static bool 
canfd_decode_reg_bank(XlnxVersalCANFDState *s, hwaddr addr,
   * @return true is the decoding succeded, false otherwise
   */
  static bool canfd_decode_addr(XlnxVersalCANFDState *s, hwaddr addr,
-                              size_t *idx, hwaddr *offset)
+                              unsigned *idx, hwaddr *offset)
  {
      if (addr <= A_RX_FIFO_WATERMARK_REGISTER) {
          /* from 0x0 to 0xec. Handled by the register API */
@@ -1852,11 +1854,10 @@ static bool 
canfd_decode_addr(XlnxVersalCANFDState *s, hwaddr addr,
  static uint64_t canfd_read(void *opaque, hwaddr addr, unsigned size)
  {
      XlnxVersalCANFDState *s = XILINX_CANFD(opaque);
-    size_t bank_idx;
      hwaddr reg_offset;
      uint64_t ret;

-    if (!canfd_decode_addr(s, addr, &bank_idx, &reg_offset)) {
+    if (!canfd_decode_addr(s, addr, NULL, &reg_offset)) {
          qemu_log_mask(LOG_GUEST_ERROR, TYPE_XILINX_CANFD
                        ": read to unknown register at address 0x%"
                        HWADDR_PRIx "\n", addr);
@@ -1875,7 +1876,7 @@ static void canfd_write(void *opaque, hwaddr addr, 
uint64_t value,
                          unsigned size)
  {
      XlnxVersalCANFDState *s = XILINX_CANFD(opaque);
-    size_t bank_idx;
+    unsigned bank_idx;
      hwaddr reg_offset;

      if (!canfd_decode_addr(s, addr, &bank_idx, &reg_offset)) {
---

... in order to avoid:

hw/net/can/xlnx-versal-canfd.c:1822:59: error: incompatible pointer 
types passing 'hwaddr *' (aka 'unsigned long long *') to parameter of 
type 'size_t *' (aka 'unsigned long *') 
[-Werror,-Wincompatible-pointer-types]
  1822 |                                      s->cfg.tx_fifo, idx, offset);
       |                                                           ^~~~~~
hw/net/can/xlnx-versal-canfd.c:1785:74: note: passing argument to 
parameter 'offset' here
  1785 |                                   size_t num_banks, size_t 
*idx, size_t *offset)
       | 
          ^
hw/net/can/xlnx-versal-canfd.c:1827:47: error: incompatible pointer 
types passing 'hwaddr *' (aka 'unsigned long long *') to parameter of 
type 'size_t *' (aka 'unsigned long *') 
[-Werror,-Wincompatible-pointer-types]
  1827 |                                      32, idx, offset);
       |                                               ^~~~~~
hw/net/can/xlnx-versal-canfd.c:1785:74: note: passing argument to 
parameter 'offset' here
  1785 |                                   size_t num_banks, size_t 
*idx, size_t *offset)
       | 
          ^
hw/net/can/xlnx-versal-canfd.c:1833:47: error: incompatible pointer 
types passing 'hwaddr *' (aka 'unsigned long long *') to parameter of 
type 'size_t *' (aka 'unsigned long *') 
[-Werror,-Wincompatible-pointer-types]
  1833 |                                      32, idx, offset);
       |                                               ^~~~~~
hw/net/can/xlnx-versal-canfd.c:1785:74: note: passing argument to 
parameter 'offset' here
  1785 |                                   size_t num_banks, size_t 
*idx, size_t *offset)
       | 
          ^
hw/net/can/xlnx-versal-canfd.c:1839:60: error: incompatible pointer 
types passing 'hwaddr *' (aka 'unsigned long long *') to parameter of 
type 'size_t *' (aka 'unsigned long *') 
[-Werror,-Wincompatible-pointer-types]
  1839 |                                      s->cfg.rx0_fifo, idx, offset);
       |                                                            ^~~~~~
hw/net/can/xlnx-versal-canfd.c:1785:74: note: passing argument to 
parameter 'offset' here
  1785 |                                   size_t num_banks, size_t 
*idx, size_t *offset)
       | 
          ^
hw/net/can/xlnx-versal-canfd.c:1845:60: error: incompatible pointer 
types passing 'hwaddr *' (aka 'unsigned long long *') to parameter of 
type 'size_t *' (aka 'unsigned long *') 
[-Werror,-Wincompatible-pointer-types]
  1845 |                                      s->cfg.rx1_fifo, idx, offset);
       |                                                            ^~~~~~
hw/net/can/xlnx-versal-canfd.c:1785:74: note: passing argument to 
parameter 'offset' here
  1785 |                                   size_t num_banks, size_t 
*idx, size_t *offset)
       | 
          ^
5 errors generated.