Re: [PATCH 0/3] single-binary: compile hw/intc/arm* files once

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 7/28/25 2:57 PM, Philippe Mathieu-Daudé wrote:
> On 28/7/25 21:34, Pierrick Bouvier wrote:
>> On 7/28/25 2:39 AM, Philippe Mathieu-Daudé wrote:
>>> On 25/7/25 22:19, Pierrick Bouvier wrote:
>>>> Move those files to hw/arm, as they depend on arm target code.
>>>>
>>>> Pierrick Bouvier (3):
>>>>      hw/arm/arm_gicv3_cpuif_common: move to hw/arm and compile only once
>>>>      hw/arm/arm_gicv3_cpuif: move to hw/arm and compile only once
>>>>      hw/arm/armv7m_nvic: move to hw/arm and compile only once
>>>>
>>>>     hw/{intc => arm}/arm_gicv3_cpuif.c        |  2 +-
>>>>     hw/{intc => arm}/arm_gicv3_cpuif_common.c |  2 +-
>>>>     hw/{intc => arm}/armv7m_nvic.c            |  0
>>>
>>> Alternatively add arm_common_ss in hw/intc/meson.build?
>>>
>>>      arm_common_ss = ss.source_set()
>>>      arm_common_ss.add(when: 'CONFIG_ARM_GIC',
>>>                        if_true: files('arm_gicv3_cpuif_common.c'))
>>>      arm_common_ss.add(when: 'CONFIG_ARM_GICV3',
>>>                        if_true: files('arm_gicv3_cpuif.c'))
>>>      arm_common_ss.add(when: 'CONFIG_ARM_V7M',
>>>                        if_true: files('armv7m_nvic.c'))
>>>      hw_common_arch += {'arm': arm_common_ss}
>>>
>>
>> The problem with this approach is that we need to aggregate hw/arm and
>> hw/intc arm related source sets, and the last line in your proposed
>> change does not have this semantic.
>> Regarding meson, hw/intc subfolder is parsed *before* hw/arm (see hw/
>> meson.build), so we can't reuse the same source set, defined in hw/arm/
>> meson.build.
>>
>> 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.
> 
> Yeah, you are right. What was suggested once was to move them to
> target/arm/hw/, being architecture dependent / specific.
> 
> $ ls -l hw/arm | wc -l
>         97
> 
> hw/arm/ contains board / soc / arm-specific hw and helpers (such
> code loaders). Nothing wrong, I'm just wondering there is room for
> improvements.
>

Sure, that makes sense.
GIC is a bit in the middle, as it's definitely an arm specific 
controller, and not a classical device. SMMU is directly in hw/arm for 
instance.

I'm really open to any structure Arm related developers would think is 
the best. It's just that moving a lot of files is always a pain for 
everyone, as it creates a lot of conflicts for people working on them 
currently, so I tried to minimize it here to target the strictly needed 
set of files only.

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.

> Personally I'd rather keep hw/arm/ for boards / soc, and move
> ARM specific components (like GIC, NVIC, SMMU, timers to target/arm/hw/.
> 
> Anyhow, I don't object to your approach :) For this series:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>>
>> I'm open to any other idea someone would have. Peter, without
>> necessarily a working solution, do you have any preference on where
>> those things should be?
>>
>>>>     hw/arm/meson.build                        |  3 +
>>>>     hw/arm/trace-events                       | 79 +++++++++++++++++++
>>>> ++++
>>>>     hw/intc/meson.build                       |  3 -
>>>>     hw/intc/trace-events                      | 79
>>>> -----------------------
>>>>     7 files changed, 84 insertions(+), 84 deletions(-)
>>>>     rename hw/{intc => arm}/arm_gicv3_cpuif.c (99%)
>>>>     rename hw/{intc => arm}/arm_gicv3_cpuif_common.c (92%)
>>>>     rename hw/{intc => arm}/armv7m_nvic.c (100%)
>>>>
>>>
>>
>