Re: [RFC PATCH 01/11] system: Extract target-specific globals to their own compilation unit

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

On 5/3/25 02:20, Pierrick Bouvier wrote:
> On 3/4/25 16:52, Philippe Mathieu-Daudé wrote:
>> We shouldn't use target specific globals for machine properties.
>> These ones could be desugarized, as explained in [*]. While
>> certainly doable, not trivial nor my priority for now. Just move
>> them to a different file to clarify they are *globals*, like the
>> generic globals residing in system/globals.c.
>>
> 
> Maybe those could be set globally to the default:
> int graphic_width = 800;
> int graphic_height = 600;
> int graphic_depth = 32;
> 
> And override them for sparc and m68k in qemu_init_arch_modules function, 
> using qemu_arch_name().
> This way, no need to have a new file to hold them.

This is not what Paolo explained ...

> 
>> [*] https://lore.kernel.org/qemu-devel/e514d6db-781d-4afe- 
>> b057-9046c70044dc@redhat.com/

... here ^, but maybe I misunderstood him.

Doing the '-g' CLI change is not my priority, and I welcome
any developer willing to help :) Here I'm just trying to make
sense of that arch_init.c file.

>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   system/arch_init.c      | 14 --------------
>>   system/globals-target.c | 24 ++++++++++++++++++++++++
>>   system/meson.build      |  1 +
>>   3 files changed, 25 insertions(+), 14 deletions(-)
>>   create mode 100644 system/globals-target.c
>>
>> diff --git a/system/arch_init.c b/system/arch_init.c
>> index d2c32f84887..ea0842b7410 100644
>> --- a/system/arch_init.c
>> +++ b/system/arch_init.c
>> @@ -25,20 +25,6 @@
>>   #include "qemu/module.h"
>>   #include "system/arch_init.h"
>> -#ifdef TARGET_SPARC
>> -int graphic_width = 1024;
>> -int graphic_height = 768;
>> -int graphic_depth = 8;
>> -#elif defined(TARGET_M68K)
>> -int graphic_width = 800;
>> -int graphic_height = 600;
>> -int graphic_depth = 8;
>> -#else
>> -int graphic_width = 800;
>> -int graphic_height = 600;
>> -int graphic_depth = 32;
>> -#endif
>> -
>>   const uint32_t arch_type = QEMU_ARCH;
>>   void qemu_init_arch_modules(void)
>> diff --git a/system/globals-target.c b/system/globals-target.c
>> new file mode 100644
>> index 00000000000..989720591e7
>> --- /dev/null
>> +++ b/system/globals-target.c
>> @@ -0,0 +1,24 @@
>> +/*
>> + * Global variables that should not exist (target specific)
>> + *
>> + * Copyright (c) 2003-2008 Fabrice Bellard
>> + *
>> + * SPDX-License-Identifier: MIT
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "system/system.h"
>> +
>> +#ifdef TARGET_SPARC
>> +int graphic_width = 1024;
>> +int graphic_height = 768;
>> +int graphic_depth = 8;
>> +#elif defined(TARGET_M68K)
>> +int graphic_width = 800;
>> +int graphic_height = 600;
>> +int graphic_depth = 8;
>> +#else
>> +int graphic_width = 800;
>> +int graphic_height = 600;
>> +int graphic_depth = 32;
>> +#endif
>> diff --git a/system/meson.build b/system/meson.build
>> index 4952f4b2c7d..181195410fb 100644
>> --- a/system/meson.build
>> +++ b/system/meson.build
>> @@ -1,6 +1,7 @@
>>   specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>>     'arch_init.c',
>>     'ioport.c',
>> +  'globals-target.c',
>>     'memory.c',
>>     'physmem.c',
>>     'watchpoint.c',
>