On Wed, Sep 17, 2025 at 01:44:41PM +0200, Luc Michel wrote:
> Based-on: 20250912100059.103997-1-luc.michel@amd.com ([PATCH v5 00/47] AMD Versal Gen 2 support)
>
> Hello,
>
> This series addresses the memory leaks caused by the register API. The
> first patches fix the API itself while the last ones take care of the
> CANFD model.
>
> The leaks in the register API were caused by:
> - the REGISTER device being initialized without being realized nor
> finalized. Those devices were not parented to anything and were
> not using the qdev API. So in the end I chose to drop the REGISTER
> object completely (patch 1).
> - Register API users not calling `register_finalize_block' on the
> RegisterInfoArray returned by `register_init_block'. Instead of
> fixing all the users, I chose to simplify the API by removing the
> need for this call. I turned the RegisterInfoArray struct into a QOM
> object and parented it to the device in `register_init_block'. This
> way it has its own finalize function that gets called when the
> parent device is finalized. This implies a small API change: the
> `register_finalize_block' function is removed (patch 2, 3, 4).
>
> The CANFD model needed special care. It was rolling out its own version
> of `register_init_block', including the discrepancies leading to the
> memory leaks. This is because the register map of this device is
> composed of main registers (from 0x0 to 0xec), followed by several banks
> of multiple registers (Tx banks, filter banks, Txe banks, ...). The
> register API is not suited for this kind of layout and the resulting
> code to handle that is convoluted. However a simple decoding logic is
> easily written for this, those registers having basically no side effect
> upon access.
>
> Patch 6 introduces this decoding logic for the banked registers, patch 7
> removes the register API bits for those, keeping the one for the base
> registers.
>
> Note: this series is based on my Versal 2 series. It modifies the CRL
> device during the register API refactoring. It can easily be rebased on
> master if needed.
>
> Thanks
>
> Luc
Hi Luc,
Entire series looks good to me!
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
>
>
> Luc Michel (7):
> 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: remove unused include directives
> hw/net/can/xlnx-versal-canfd: refactor the banked registers logic
> hw/net/can/xlnx-versal-canfd: remove register API usage for banked
> regs
>
> include/hw/misc/xlnx-versal-crl.h | 1 -
> include/hw/misc/xlnx-versal-xramc.h | 1 -
> include/hw/misc/xlnx-zynqmp-apu-ctrl.h | 1 -
> include/hw/misc/xlnx-zynqmp-crf.h | 1 -
> include/hw/net/xlnx-versal-canfd.h | 8 -
> include/hw/nvram/xlnx-bbram.h | 1 -
> include/hw/register.h | 25 +-
> hw/core/register.c | 36 +-
> hw/misc/xlnx-versal-crl.c | 38 +--
> hw/misc/xlnx-versal-trng.c | 1 -
> hw/misc/xlnx-versal-xramc.c | 12 +-
> hw/misc/xlnx-zynqmp-apu-ctrl.c | 12 +-
> hw/misc/xlnx-zynqmp-crf.c | 12 +-
> hw/net/can/xlnx-versal-canfd.c | 434 +++++++++----------------
> hw/nvram/xlnx-bbram.c | 13 +-
> hw/nvram/xlnx-versal-efuse-ctrl.c | 1 -
> hw/nvram/xlnx-zynqmp-efuse.c | 8 -
> 17 files changed, 196 insertions(+), 409 deletions(-)
>
> --
> 2.50.1
>