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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.

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%)
>>>
>>
>