On Wed, Sep 17, 2025 at 01:44:41PM +0200, Luc Michel wrote: > Based-on: [email protected] ([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 <[email protected]> > > > 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 >
