From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>,
qemu-devel@nongnu.org, qemu-arm@nongnu.org
Subject: Re: [PATCH 0/3] single-binary: compile hw/intc/arm* files once
Date: Thu, 31 Jul 2025 11:30:19 -0700 [thread overview]
Message-ID: <25fe9c70-be00-4884-8d91-53dfff745cb6@linaro.org> (raw)
In-Reply-To: <CAFEAcA-ycO=AO8UO+X3f6pw34J=iFwW3dpPyMXKm5BzJ11dt5Q@mail.gmail.com>
On 7/31/25 9:23 AM, Peter Maydell wrote:
> On Mon, 28 Jul 2025 at 20:34, Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>> This old commit (7702e47c2) was the origin of having interrupt related
>> code in a generic folder, but I don't really understand the rationale
>> behind it to be honest. It seems to be an exception regarding all the
>> rest of the codebase, thus the idea to bring back things where they belong.
>
> Most devices are both (a) architecture specific and (b) a particular
> kind of device (UART, ethernet controller, interrupt controller, etc).
> The nature of a filesystem hierarchy is that we can't file them
> in both ways at once. We picked "sort them by kind", which is why
> all the interrupt controllers live in hw/intc, all the UARTS in
> hw/char, ethernet controllers in hw/net, and so on. In this
> breakdown of the world, hw/$ARCH is supposed to be for board models
> and SoC models only.
>
> The GICv3 and the NVIC are odd, because they are very closely
> coupled to the CPU. (A few other interrupt controllers are also
> like this, but many are not: for instance the GICv2 is a distinct
> bit of hardware that communicates with the CPU over the IRQ and
> FIQ lines only.)
>
> One of my post-implementation regrets about GICv3 is that we
> didn't really get the split between the GICv3 proper and its
> CPU interface right. In hardware the GICv3 is an external device
> and the CPU interface is part of the CPU, with a defined
> protocol for talking between them. In QEMU we put all the
> implementation of this in hw/intc/, and the code in arm_gicv3_cpuif.c
> does some ad-hoc installing of hooks into the CPU.
>
> For the GICv5 I'm trying to structure this in a cleaner way that
> is closer to the hardware structure, so the CPU interface
> will be code in target/arm/, with a clearly defined set of
> functions that it calls to talk to the rest of the GIC that
> lives in hw/intc/. (This would be too much upheaval to
> retrofit to GICv3 though, I think.)
>
> In a green-field design of M-profile we might have made
> the NVIC be code in target/arm, and instead of a separate
> device have the CPU object itself do this code. But at the
> time it was written we didn't have the same QOM device
> class setup we did at the time, and IIRC CPU objects
> weren't a subclass of device.
>
Thanks for your answer Peter, it makes more clear for me what is the
rationale between sorting the devices this way. It seems the root issue
is the lack of proper interfacing between target cpu, and devices
relying on it.
I don't expect any silver bullet to solve this, but we still need to
move forward, so I'll share some options below.
>> As well, I would prefer having a clean build system more than a clear
>> filesystem structure, considering it's quite easy to jump into any
>> definition automatically with your work editor nowadays, vs understand a
>> meson.build file full of tricks and implicit dependencies where no tool
>> can help you.
>
> On the other hand, I prefer to have the source files in
> a clear structure, because then you know where to find
> things, and command line tools like grep etc are easier
> to use. (I don't use editor jump-to-definition: I've never
> felt the need to try to set it up.) Build system files on the
> other hand are things that most people don't need to look at
> or do more than very simple "add another file in the same pattern
> as the existing ones", so it's not too bad if they accumulate
> a little complexity.
>
This maybe explains why QEMU is a bit messy regarding its build system
architecture, because people are not interested into it. IMHO it's a
mistake, because a clean build system architecture will usually force a
clean software architecture, at least in terms of components and
interfacing. This is what we see right now, with some of the fixes from
the single binary being to extract proper API with fixed types, that
allow components to communicate in a proper way.
Complexity does not help neither, because it makes meson build files
hard to understand, and probably push back a lot of people from looking
at this. It's sad considering meson first objective is precisely to
limit the complexity of build systems.
Regarding the "modern" completion support, I recommend you take a look
at it. Even though you wrote or reviewed most of the code you navigate
in everyday, and thus don't need it, it has become a standard tool for
any developer, like sanitizers or omniscient debugging. It's especially
interesting since those tools are based on compilers (clangd is the
standard for C/C++ nowadays) and not a bunch of clunky regexps.
It's even more interesting when you learn a new language, like Rust.
> Looking at hw/intc, there is a lot of use of specific_ss
> here, so I suspect that these Arm interrupt controllers are
> not going to be the only ones that are using target-dependent
> code (there are 25 files which use CPUState, for instance).
> So I think it's worth figuring out how to build these in
> the right way where they are rather than saying that
> various interrupt controller models should move to
> a place where they don't logically belong because that happens
> to be a folder where we have the build machinery for it.
>
Coming back to our issue, I can see two ways to solve it in a short term:
- On build system: define target hw before generic ones, so we can reuse
all the source sets defined there. This has the advantage to be usable
by all others architectures.
- Move gic related fields to a substructure in arm cpu, and provide a
simple accessor to it, like "cpu_gicv3(cpu)", breaking the dependency to
cpu.h. Concerned fields would be: gic_num_lrs, gic_vpribits,
gic_vprebits, gic_pribits.
As you'll be the one having the final word and merging this, which
option would you prefer to see?
> thanks
> -- PMM
next prev parent reply other threads:[~2025-07-31 20:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-25 20:19 [PATCH 0/3] single-binary: compile hw/intc/arm* files once Pierrick Bouvier
2025-07-25 20:19 ` [PATCH 1/3] hw/arm/arm_gicv3_cpuif_common: move to hw/arm and compile only once Pierrick Bouvier
2025-07-25 20:19 ` [PATCH 2/3] hw/arm/arm_gicv3_cpuif: " Pierrick Bouvier
2025-07-25 20:19 ` [PATCH 3/3] hw/arm/armv7m_nvic: " Pierrick Bouvier
2025-07-28 9:39 ` [PATCH 0/3] single-binary: compile hw/intc/arm* files once Philippe Mathieu-Daudé
2025-07-28 19:34 ` Pierrick Bouvier
2025-07-28 21:57 ` Philippe Mathieu-Daudé
2025-07-28 22:04 ` Pierrick Bouvier
2025-07-31 16:23 ` Peter Maydell
2025-07-31 18:30 ` Pierrick Bouvier [this message]
2025-07-31 21:27 ` Philippe Mathieu-Daudé
2025-07-31 22:24 ` Pierrick Bouvier
2025-08-01 8:34 ` Peter Maydell
2025-08-01 16:31 ` Pierrick Bouvier
2025-08-01 16:38 ` Peter Maydell
2025-08-01 16:59 ` Pierrick Bouvier
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=25fe9c70-be00-4884-8d91-53dfff745cb6@linaro.org \
--to=pierrick.bouvier@linaro.org \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/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).