From: Luc Michel <luc.michel@amd.com>
To: <qemu-devel@nongnu.org>, <qemu-arm@nongnu.org>
Cc: "Luc Michel" <luc.michel@amd.com>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Francisco Iglesias" <francisco.iglesias@amd.com>,
"Edgar E . Iglesias" <edgar.iglesias@amd.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Alistair Francis" <alistair@alistair23.me>,
"Vikram Garhwal" <vikram.garhwal@bytedance.com>
Subject: [PATCH v2 0/7] Register API leaks fixes
Date: Thu, 2 Oct 2025 09:34:09 +0200 [thread overview]
Message-ID: <20251002073418.109375-1-luc.michel@amd.com> (raw)
Based-on: 20250926070806.292065-1-luc.michel@amd.com ([PATCH v6 00/47] AMD Versal Gen 2 support)
v2:
- Rebased, now applies cleanly. The conflict came from f9ce0848942490
that was merged in the mean time.
As discussed with Phil, this is still based on Versal 2 series to
avoid having to rebase the later on this one, as it is basically
ready to be merged. [Phil]
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
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.51.0
next reply other threads:[~2025-10-02 7:36 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-02 7:34 Luc Michel [this message]
2025-10-02 7:34 ` [PATCH v2 1/7] hw/core/register: remove the REGISTER device type Luc Michel
2025-10-02 7:34 ` [PATCH v2 2/7] hw/core/register: add the REGISTER_ARRAY type Luc Michel
2025-10-02 7:34 ` [PATCH v2 3/7] hw/core/register: remove the calls to `register_finalize_block' Luc Michel
2025-10-02 7:34 ` [PATCH v2 4/7] hw/core/register: remove the `register_finalize_block' function Luc Michel
2025-10-02 7:34 ` [PATCH v2 5/7] hw/net/can/xlnx-versal-canfd: remove unused include directives Luc Michel
2025-10-02 7:34 ` [PATCH v2 6/7] hw/net/can/xlnx-versal-canfd: refactor the banked registers logic Luc Michel
2025-10-02 7:34 ` [PATCH v2 7/7] hw/net/can/xlnx-versal-canfd: remove register API usage for banked regs Luc Michel
2025-10-09 6:23 ` [PATCH v2 0/7] Register API leaks fixes Philippe Mathieu-Daudé
2025-10-09 6:33 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251002073418.109375-1-luc.michel@amd.com \
--to=luc.michel@amd.com \
--cc=alistair@alistair23.me \
--cc=edgar.iglesias@amd.com \
--cc=francisco.iglesias@amd.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vikram.garhwal@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).