| Message ID | 20250305005225.95051-4-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qemu: Remove TARGET_NAME definition | expand |
On 3/4/25 16:52, Philippe Mathieu-Daudé wrote: > Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. > Use them to declare QEMU_ARCH_$target bitmasks. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > meson.build | 4 +-- > include/system/arch_init.h | 65 +++++++++++++++++++++++++------------- > system/arch_init.c | 2 +- > 3 files changed, 46 insertions(+), 25 deletions(-) > > diff --git a/meson.build b/meson.build > index 0a2c61d2bfa..1ab02a5d48d 100644 > --- a/meson.build > +++ b/meson.build > @@ -3357,8 +3357,8 @@ foreach target : target_dirs > config_target_data.set(k, v) > endif > endforeach > - config_target_data.set('QEMU_ARCH', > - 'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper()) > + config_target_data.set('QEMU_ARCH_BIT', > + 'QEMU_ARCH_BIT_' + config_target['TARGET_BASE_ARCH'].to_upper()) > config_target_h += {target: configure_file(output: target + '-config-target.h', > configuration: config_target_data)} > > diff --git a/include/system/arch_init.h b/include/system/arch_init.h > index d8b77440487..06e5527ec88 100644 > --- a/include/system/arch_init.h > +++ b/include/system/arch_init.h > @@ -1,29 +1,50 @@ > #ifndef QEMU_ARCH_INIT_H > #define QEMU_ARCH_INIT_H > > +#include "qemu/bitops.h" > > -enum { > - QEMU_ARCH_ALL = -1, > - QEMU_ARCH_ALPHA = (1 << 0), > - QEMU_ARCH_ARM = (1 << 1), > - QEMU_ARCH_I386 = (1 << 3), > - QEMU_ARCH_M68K = (1 << 4), > - QEMU_ARCH_MICROBLAZE = (1 << 6), > - QEMU_ARCH_MIPS = (1 << 7), > - QEMU_ARCH_PPC = (1 << 8), > - QEMU_ARCH_S390X = (1 << 9), > - QEMU_ARCH_SH4 = (1 << 10), > - QEMU_ARCH_SPARC = (1 << 11), > - QEMU_ARCH_XTENSA = (1 << 12), > - QEMU_ARCH_OPENRISC = (1 << 13), > - QEMU_ARCH_TRICORE = (1 << 16), > - QEMU_ARCH_HPPA = (1 << 18), > - QEMU_ARCH_RISCV = (1 << 19), > - QEMU_ARCH_RX = (1 << 20), > - QEMU_ARCH_AVR = (1 << 21), > - QEMU_ARCH_HEXAGON = (1 << 22), > - QEMU_ARCH_LOONGARCH = (1 << 23), > -}; > +typedef enum QemuArchBit { > + QEMU_ARCH_BIT_ALPHA = 0, > + QEMU_ARCH_BIT_ARM = 1, > + QEMU_ARCH_BIT_I386 = 3, > + QEMU_ARCH_BIT_M68K = 4, > + QEMU_ARCH_BIT_MICROBLAZE = 6, > + QEMU_ARCH_BIT_MIPS = 7, > + QEMU_ARCH_BIT_PPC = 8, > + QEMU_ARCH_BIT_S390X = 9, > + QEMU_ARCH_BIT_SH4 = 10, > + QEMU_ARCH_BIT_SPARC = 11, > + QEMU_ARCH_BIT_XTENSA = 12, > + QEMU_ARCH_BIT_OPENRISC = 13, > + QEMU_ARCH_BIT_TRICORE = 16, > + QEMU_ARCH_BIT_HPPA = 18, > + QEMU_ARCH_BIT_RISCV = 19, > + QEMU_ARCH_BIT_RX = 20, > + QEMU_ARCH_BIT_AVR = 21, > + QEMU_ARCH_BIT_HEXAGON = 22, > + QEMU_ARCH_BIT_LOONGARCH = 23, > +} QemuArchBit; > + > +#define QEMU_ARCH_ALPHA BIT(QEMU_ARCH_BIT_ALPHA) > +#define QEMU_ARCH_ARM BIT(QEMU_ARCH_BIT_ARM) > +#define QEMU_ARCH_I386 BIT(QEMU_ARCH_BIT_I386) > +#define QEMU_ARCH_M68K BIT(QEMU_ARCH_BIT_M68K) > +#define QEMU_ARCH_MICROBLAZE BIT(QEMU_ARCH_BIT_MICROBLAZE) > +#define QEMU_ARCH_MIPS BIT(QEMU_ARCH_BIT_MIPS) > +#define QEMU_ARCH_PPC BIT(QEMU_ARCH_BIT_PPC) > +#define QEMU_ARCH_S390X BIT(QEMU_ARCH_BIT_S390X) > +#define QEMU_ARCH_SH4 BIT(QEMU_ARCH_BIT_SH4) > +#define QEMU_ARCH_SPARC BIT(QEMU_ARCH_BIT_SPARC) > +#define QEMU_ARCH_XTENSA BIT(QEMU_ARCH_BIT_XTENSA) > +#define QEMU_ARCH_OPENRISC BIT(QEMU_ARCH_BIT_OPENRISC) > +#define QEMU_ARCH_TRICORE BIT(QEMU_ARCH_BIT_TRICORE) > +#define QEMU_ARCH_HPPA BIT(QEMU_ARCH_BIT_HPPA) > +#define QEMU_ARCH_RISCV BIT(QEMU_ARCH_BIT_RISCV) > +#define QEMU_ARCH_RX BIT(QEMU_ARCH_BIT_RX) > +#define QEMU_ARCH_AVR BIT(QEMU_ARCH_BIT_AVR) > +#define QEMU_ARCH_HEXAGON BIT(QEMU_ARCH_BIT_HEXAGON) > +#define QEMU_ARCH_LOONGARCH BIT(QEMU_ARCH_BIT_LOONGARCH) > +#define QEMU_ARCH_ALL -1 > > extern const uint32_t arch_type; > What are we gaining by having a "bit" oriented enum, vs simple values that can be compared too? As well, it would make sense to add subvariants (aarch64, x86_64, little and big endian variants for some arch), so all of them are present and can be queried easily. > diff --git a/system/arch_init.c b/system/arch_init.c > index b9147af93cb..fedbb18e2cc 100644 > --- a/system/arch_init.c > +++ b/system/arch_init.c > @@ -24,4 +24,4 @@ > #include "qemu/osdep.h" > #include "system/arch_init.h" > > -const uint32_t arch_type = QEMU_ARCH; > +const uint32_t arch_type = BIT(QEMU_ARCH_BIT);
On 5/3/25 02:23, Pierrick Bouvier wrote: > On 3/4/25 16:52, Philippe Mathieu-Daudé wrote: >> Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. >> Use them to declare QEMU_ARCH_$target bitmasks. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> meson.build | 4 +-- >> include/system/arch_init.h | 65 +++++++++++++++++++++++++------------- >> system/arch_init.c | 2 +- >> 3 files changed, 46 insertions(+), 25 deletions(-) >> >> diff --git a/meson.build b/meson.build >> index 0a2c61d2bfa..1ab02a5d48d 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -3357,8 +3357,8 @@ foreach target : target_dirs >> config_target_data.set(k, v) >> endif >> endforeach >> - config_target_data.set('QEMU_ARCH', >> - 'QEMU_ARCH_' + >> config_target['TARGET_BASE_ARCH'].to_upper()) >> + config_target_data.set('QEMU_ARCH_BIT', >> + 'QEMU_ARCH_BIT_' + >> config_target['TARGET_BASE_ARCH'].to_upper()) >> config_target_h += {target: configure_file(output: target + '- >> config-target.h', >> configuration: >> config_target_data)} >> diff --git a/include/system/arch_init.h b/include/system/arch_init.h >> index d8b77440487..06e5527ec88 100644 >> --- a/include/system/arch_init.h >> +++ b/include/system/arch_init.h >> @@ -1,29 +1,50 @@ >> #ifndef QEMU_ARCH_INIT_H >> #define QEMU_ARCH_INIT_H >> +#include "qemu/bitops.h" >> -enum { >> - QEMU_ARCH_ALL = -1, >> - QEMU_ARCH_ALPHA = (1 << 0), >> - QEMU_ARCH_ARM = (1 << 1), >> - QEMU_ARCH_I386 = (1 << 3), >> - QEMU_ARCH_M68K = (1 << 4), >> - QEMU_ARCH_MICROBLAZE = (1 << 6), >> - QEMU_ARCH_MIPS = (1 << 7), >> - QEMU_ARCH_PPC = (1 << 8), >> - QEMU_ARCH_S390X = (1 << 9), >> - QEMU_ARCH_SH4 = (1 << 10), >> - QEMU_ARCH_SPARC = (1 << 11), >> - QEMU_ARCH_XTENSA = (1 << 12), >> - QEMU_ARCH_OPENRISC = (1 << 13), >> - QEMU_ARCH_TRICORE = (1 << 16), >> - QEMU_ARCH_HPPA = (1 << 18), >> - QEMU_ARCH_RISCV = (1 << 19), >> - QEMU_ARCH_RX = (1 << 20), >> - QEMU_ARCH_AVR = (1 << 21), >> - QEMU_ARCH_HEXAGON = (1 << 22), >> - QEMU_ARCH_LOONGARCH = (1 << 23), >> -}; >> +typedef enum QemuArchBit { >> + QEMU_ARCH_BIT_ALPHA = 0, >> + QEMU_ARCH_BIT_ARM = 1, >> + QEMU_ARCH_BIT_I386 = 3, >> + QEMU_ARCH_BIT_M68K = 4, >> + QEMU_ARCH_BIT_MICROBLAZE = 6, >> + QEMU_ARCH_BIT_MIPS = 7, >> + QEMU_ARCH_BIT_PPC = 8, >> + QEMU_ARCH_BIT_S390X = 9, >> + QEMU_ARCH_BIT_SH4 = 10, >> + QEMU_ARCH_BIT_SPARC = 11, >> + QEMU_ARCH_BIT_XTENSA = 12, >> + QEMU_ARCH_BIT_OPENRISC = 13, >> + QEMU_ARCH_BIT_TRICORE = 16, >> + QEMU_ARCH_BIT_HPPA = 18, >> + QEMU_ARCH_BIT_RISCV = 19, >> + QEMU_ARCH_BIT_RX = 20, >> + QEMU_ARCH_BIT_AVR = 21, >> + QEMU_ARCH_BIT_HEXAGON = 22, >> + QEMU_ARCH_BIT_LOONGARCH = 23, >> +} QemuArchBit; >> + >> +#define QEMU_ARCH_ALPHA BIT(QEMU_ARCH_BIT_ALPHA) >> +#define QEMU_ARCH_ARM BIT(QEMU_ARCH_BIT_ARM) >> +#define QEMU_ARCH_I386 BIT(QEMU_ARCH_BIT_I386) >> +#define QEMU_ARCH_M68K BIT(QEMU_ARCH_BIT_M68K) >> +#define QEMU_ARCH_MICROBLAZE BIT(QEMU_ARCH_BIT_MICROBLAZE) >> +#define QEMU_ARCH_MIPS BIT(QEMU_ARCH_BIT_MIPS) >> +#define QEMU_ARCH_PPC BIT(QEMU_ARCH_BIT_PPC) >> +#define QEMU_ARCH_S390X BIT(QEMU_ARCH_BIT_S390X) >> +#define QEMU_ARCH_SH4 BIT(QEMU_ARCH_BIT_SH4) >> +#define QEMU_ARCH_SPARC BIT(QEMU_ARCH_BIT_SPARC) >> +#define QEMU_ARCH_XTENSA BIT(QEMU_ARCH_BIT_XTENSA) >> +#define QEMU_ARCH_OPENRISC BIT(QEMU_ARCH_BIT_OPENRISC) >> +#define QEMU_ARCH_TRICORE BIT(QEMU_ARCH_BIT_TRICORE) >> +#define QEMU_ARCH_HPPA BIT(QEMU_ARCH_BIT_HPPA) >> +#define QEMU_ARCH_RISCV BIT(QEMU_ARCH_BIT_RISCV) >> +#define QEMU_ARCH_RX BIT(QEMU_ARCH_BIT_RX) >> +#define QEMU_ARCH_AVR BIT(QEMU_ARCH_BIT_AVR) >> +#define QEMU_ARCH_HEXAGON BIT(QEMU_ARCH_BIT_HEXAGON) >> +#define QEMU_ARCH_LOONGARCH BIT(QEMU_ARCH_BIT_LOONGARCH) >> +#define QEMU_ARCH_ALL -1 >> extern const uint32_t arch_type; > > What are we gaining by having a "bit" oriented enum, vs simple values > that can be compared too? I'm not sure what you are asking here, these definitions are heavily used in qemu-options.hx, and I don't plan to change them anytime soon. For the single binary I'll try to keep the command line interface with no change. For heterogeneous binary I plan to start with no CLI so I'm not really considering them. > As well, it would make sense to add subvariants (aarch64, x86_64, little > and big endian variants for some arch), so all of them are present and > can be queried easily. IIUC what you are referring, this is planned for another interface, but not this series which is focused on introducing qemu_arch_name() and removing TARGET_NAME. While you don't see any improvement in duplicated target files as of this series, some reduction should happen in the next step which remove TARGET_BIG_ENDIAN uses in hw/. >> diff --git a/system/arch_init.c b/system/arch_init.c >> index b9147af93cb..fedbb18e2cc 100644 >> --- a/system/arch_init.c >> +++ b/system/arch_init.c >> @@ -24,4 +24,4 @@ >> #include "qemu/osdep.h" >> #include "system/arch_init.h" >> -const uint32_t arch_type = QEMU_ARCH; >> +const uint32_t arch_type = BIT(QEMU_ARCH_BIT); >
On 3/4/25 17:31, Philippe Mathieu-Daudé wrote: > On 5/3/25 02:23, Pierrick Bouvier wrote: >> On 3/4/25 16:52, Philippe Mathieu-Daudé wrote: >>> Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. >>> Use them to declare QEMU_ARCH_$target bitmasks. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> meson.build | 4 +-- >>> include/system/arch_init.h | 65 +++++++++++++++++++++++++------------- >>> system/arch_init.c | 2 +- >>> 3 files changed, 46 insertions(+), 25 deletions(-) >>> >>> diff --git a/meson.build b/meson.build >>> index 0a2c61d2bfa..1ab02a5d48d 100644 >>> --- a/meson.build >>> +++ b/meson.build >>> @@ -3357,8 +3357,8 @@ foreach target : target_dirs >>> config_target_data.set(k, v) >>> endif >>> endforeach >>> - config_target_data.set('QEMU_ARCH', >>> - 'QEMU_ARCH_' + >>> config_target['TARGET_BASE_ARCH'].to_upper()) >>> + config_target_data.set('QEMU_ARCH_BIT', >>> + 'QEMU_ARCH_BIT_' + >>> config_target['TARGET_BASE_ARCH'].to_upper()) >>> config_target_h += {target: configure_file(output: target + '- >>> config-target.h', >>> configuration: >>> config_target_data)} >>> diff --git a/include/system/arch_init.h b/include/system/arch_init.h >>> index d8b77440487..06e5527ec88 100644 >>> --- a/include/system/arch_init.h >>> +++ b/include/system/arch_init.h >>> @@ -1,29 +1,50 @@ >>> #ifndef QEMU_ARCH_INIT_H >>> #define QEMU_ARCH_INIT_H >>> +#include "qemu/bitops.h" >>> -enum { >>> - QEMU_ARCH_ALL = -1, >>> - QEMU_ARCH_ALPHA = (1 << 0), >>> - QEMU_ARCH_ARM = (1 << 1), >>> - QEMU_ARCH_I386 = (1 << 3), >>> - QEMU_ARCH_M68K = (1 << 4), >>> - QEMU_ARCH_MICROBLAZE = (1 << 6), >>> - QEMU_ARCH_MIPS = (1 << 7), >>> - QEMU_ARCH_PPC = (1 << 8), >>> - QEMU_ARCH_S390X = (1 << 9), >>> - QEMU_ARCH_SH4 = (1 << 10), >>> - QEMU_ARCH_SPARC = (1 << 11), >>> - QEMU_ARCH_XTENSA = (1 << 12), >>> - QEMU_ARCH_OPENRISC = (1 << 13), >>> - QEMU_ARCH_TRICORE = (1 << 16), >>> - QEMU_ARCH_HPPA = (1 << 18), >>> - QEMU_ARCH_RISCV = (1 << 19), >>> - QEMU_ARCH_RX = (1 << 20), >>> - QEMU_ARCH_AVR = (1 << 21), >>> - QEMU_ARCH_HEXAGON = (1 << 22), >>> - QEMU_ARCH_LOONGARCH = (1 << 23), >>> -}; >>> +typedef enum QemuArchBit { >>> + QEMU_ARCH_BIT_ALPHA = 0, >>> + QEMU_ARCH_BIT_ARM = 1, >>> + QEMU_ARCH_BIT_I386 = 3, >>> + QEMU_ARCH_BIT_M68K = 4, >>> + QEMU_ARCH_BIT_MICROBLAZE = 6, >>> + QEMU_ARCH_BIT_MIPS = 7, >>> + QEMU_ARCH_BIT_PPC = 8, >>> + QEMU_ARCH_BIT_S390X = 9, >>> + QEMU_ARCH_BIT_SH4 = 10, >>> + QEMU_ARCH_BIT_SPARC = 11, >>> + QEMU_ARCH_BIT_XTENSA = 12, >>> + QEMU_ARCH_BIT_OPENRISC = 13, >>> + QEMU_ARCH_BIT_TRICORE = 16, >>> + QEMU_ARCH_BIT_HPPA = 18, >>> + QEMU_ARCH_BIT_RISCV = 19, >>> + QEMU_ARCH_BIT_RX = 20, >>> + QEMU_ARCH_BIT_AVR = 21, >>> + QEMU_ARCH_BIT_HEXAGON = 22, >>> + QEMU_ARCH_BIT_LOONGARCH = 23, >>> +} QemuArchBit; >>> + >>> +#define QEMU_ARCH_ALPHA BIT(QEMU_ARCH_BIT_ALPHA) >>> +#define QEMU_ARCH_ARM BIT(QEMU_ARCH_BIT_ARM) >>> +#define QEMU_ARCH_I386 BIT(QEMU_ARCH_BIT_I386) >>> +#define QEMU_ARCH_M68K BIT(QEMU_ARCH_BIT_M68K) >>> +#define QEMU_ARCH_MICROBLAZE BIT(QEMU_ARCH_BIT_MICROBLAZE) >>> +#define QEMU_ARCH_MIPS BIT(QEMU_ARCH_BIT_MIPS) >>> +#define QEMU_ARCH_PPC BIT(QEMU_ARCH_BIT_PPC) >>> +#define QEMU_ARCH_S390X BIT(QEMU_ARCH_BIT_S390X) >>> +#define QEMU_ARCH_SH4 BIT(QEMU_ARCH_BIT_SH4) >>> +#define QEMU_ARCH_SPARC BIT(QEMU_ARCH_BIT_SPARC) >>> +#define QEMU_ARCH_XTENSA BIT(QEMU_ARCH_BIT_XTENSA) >>> +#define QEMU_ARCH_OPENRISC BIT(QEMU_ARCH_BIT_OPENRISC) >>> +#define QEMU_ARCH_TRICORE BIT(QEMU_ARCH_BIT_TRICORE) >>> +#define QEMU_ARCH_HPPA BIT(QEMU_ARCH_BIT_HPPA) >>> +#define QEMU_ARCH_RISCV BIT(QEMU_ARCH_BIT_RISCV) >>> +#define QEMU_ARCH_RX BIT(QEMU_ARCH_BIT_RX) >>> +#define QEMU_ARCH_AVR BIT(QEMU_ARCH_BIT_AVR) >>> +#define QEMU_ARCH_HEXAGON BIT(QEMU_ARCH_BIT_HEXAGON) >>> +#define QEMU_ARCH_LOONGARCH BIT(QEMU_ARCH_BIT_LOONGARCH) >>> +#define QEMU_ARCH_ALL -1 >>> extern const uint32_t arch_type; >> >> What are we gaining by having a "bit" oriented enum, vs simple values >> that can be compared too? > > I'm not sure what you are asking here, these definitions are heavily > used in qemu-options.hx, and I don't plan to change them anytime soon. > > For the single binary I'll try to keep the command line interface with > no change. For heterogeneous binary I plan to start with no CLI so I'm > not really considering them. > My bad, I thought we were introducing something new here. Yes, let's stick to a minimal change. >> As well, it would make sense to add subvariants (aarch64, x86_64, little >> and big endian variants for some arch), so all of them are present and >> can be queried easily. > > IIUC what you are referring, this is planned for another interface, but > not this series which is focused on introducing qemu_arch_name() and > removing TARGET_NAME. While you don't see any improvement in duplicated > target files as of this series, some reduction should happen in the next > step which remove TARGET_BIG_ENDIAN uses in hw/. > Yes, sounds good. It's just a bit hard to track the changes between the cleanup of existing files, and introducing a new API to query this information at run time. >>> diff --git a/system/arch_init.c b/system/arch_init.c >>> index b9147af93cb..fedbb18e2cc 100644 >>> --- a/system/arch_init.c >>> +++ b/system/arch_init.c >>> @@ -24,4 +24,4 @@ >>> #include "qemu/osdep.h" >>> #include "system/arch_init.h" >>> -const uint32_t arch_type = QEMU_ARCH; >>> +const uint32_t arch_type = BIT(QEMU_ARCH_BIT); >> >
On 5/3/25 02:36, Pierrick Bouvier wrote: > On 3/4/25 17:31, Philippe Mathieu-Daudé wrote: >> On 5/3/25 02:23, Pierrick Bouvier wrote: >>> On 3/4/25 16:52, Philippe Mathieu-Daudé wrote: >>>> Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. >>>> Use them to declare QEMU_ARCH_$target bitmasks. >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>> --- >>>> meson.build | 4 +-- >>>> include/system/arch_init.h | 65 ++++++++++++++++++++++++ >>>> +------------- >>>> system/arch_init.c | 2 +- >>>> 3 files changed, 46 insertions(+), 25 deletions(-) >>>> >>>> diff --git a/meson.build b/meson.build >>>> index 0a2c61d2bfa..1ab02a5d48d 100644 >>>> --- a/meson.build >>>> +++ b/meson.build >>>> @@ -3357,8 +3357,8 @@ foreach target : target_dirs >>>> config_target_data.set(k, v) >>>> endif >>>> endforeach >>>> - config_target_data.set('QEMU_ARCH', >>>> - 'QEMU_ARCH_' + >>>> config_target['TARGET_BASE_ARCH'].to_upper()) >>>> + config_target_data.set('QEMU_ARCH_BIT', >>>> + 'QEMU_ARCH_BIT_' + >>>> config_target['TARGET_BASE_ARCH'].to_upper()) >>>> config_target_h += {target: configure_file(output: target + '- >>>> config-target.h', >>>> configuration: >>>> config_target_data)} >>>> diff --git a/include/system/arch_init.h b/include/system/arch_init.h >>>> index d8b77440487..06e5527ec88 100644 >>>> --- a/include/system/arch_init.h >>>> +++ b/include/system/arch_init.h >>>> @@ -1,29 +1,50 @@ >>>> #ifndef QEMU_ARCH_INIT_H >>>> #define QEMU_ARCH_INIT_H >>>> +#include "qemu/bitops.h" >>>> -enum { >>>> - QEMU_ARCH_ALL = -1, >>>> - QEMU_ARCH_ALPHA = (1 << 0), >>>> - QEMU_ARCH_ARM = (1 << 1), >>>> - QEMU_ARCH_I386 = (1 << 3), >>>> - QEMU_ARCH_M68K = (1 << 4), >>>> - QEMU_ARCH_MICROBLAZE = (1 << 6), >>>> - QEMU_ARCH_MIPS = (1 << 7), >>>> - QEMU_ARCH_PPC = (1 << 8), >>>> - QEMU_ARCH_S390X = (1 << 9), >>>> - QEMU_ARCH_SH4 = (1 << 10), >>>> - QEMU_ARCH_SPARC = (1 << 11), >>>> - QEMU_ARCH_XTENSA = (1 << 12), >>>> - QEMU_ARCH_OPENRISC = (1 << 13), >>>> - QEMU_ARCH_TRICORE = (1 << 16), >>>> - QEMU_ARCH_HPPA = (1 << 18), >>>> - QEMU_ARCH_RISCV = (1 << 19), >>>> - QEMU_ARCH_RX = (1 << 20), >>>> - QEMU_ARCH_AVR = (1 << 21), >>>> - QEMU_ARCH_HEXAGON = (1 << 22), >>>> - QEMU_ARCH_LOONGARCH = (1 << 23), >>>> -}; >>>> +typedef enum QemuArchBit { >>>> + QEMU_ARCH_BIT_ALPHA = 0, >>>> + QEMU_ARCH_BIT_ARM = 1, >>>> + QEMU_ARCH_BIT_I386 = 3, >>>> + QEMU_ARCH_BIT_M68K = 4, >>>> + QEMU_ARCH_BIT_MICROBLAZE = 6, >>>> + QEMU_ARCH_BIT_MIPS = 7, >>>> + QEMU_ARCH_BIT_PPC = 8, >>>> + QEMU_ARCH_BIT_S390X = 9, >>>> + QEMU_ARCH_BIT_SH4 = 10, >>>> + QEMU_ARCH_BIT_SPARC = 11, >>>> + QEMU_ARCH_BIT_XTENSA = 12, >>>> + QEMU_ARCH_BIT_OPENRISC = 13, >>>> + QEMU_ARCH_BIT_TRICORE = 16, >>>> + QEMU_ARCH_BIT_HPPA = 18, >>>> + QEMU_ARCH_BIT_RISCV = 19, >>>> + QEMU_ARCH_BIT_RX = 20, >>>> + QEMU_ARCH_BIT_AVR = 21, >>>> + QEMU_ARCH_BIT_HEXAGON = 22, >>>> + QEMU_ARCH_BIT_LOONGARCH = 23, >>>> +} QemuArchBit; >>>> + >>>> +#define QEMU_ARCH_ALPHA BIT(QEMU_ARCH_BIT_ALPHA) >>>> +#define QEMU_ARCH_ARM BIT(QEMU_ARCH_BIT_ARM) >>>> +#define QEMU_ARCH_I386 BIT(QEMU_ARCH_BIT_I386) >>>> +#define QEMU_ARCH_M68K BIT(QEMU_ARCH_BIT_M68K) >>>> +#define QEMU_ARCH_MICROBLAZE BIT(QEMU_ARCH_BIT_MICROBLAZE) >>>> +#define QEMU_ARCH_MIPS BIT(QEMU_ARCH_BIT_MIPS) >>>> +#define QEMU_ARCH_PPC BIT(QEMU_ARCH_BIT_PPC) >>>> +#define QEMU_ARCH_S390X BIT(QEMU_ARCH_BIT_S390X) >>>> +#define QEMU_ARCH_SH4 BIT(QEMU_ARCH_BIT_SH4) >>>> +#define QEMU_ARCH_SPARC BIT(QEMU_ARCH_BIT_SPARC) >>>> +#define QEMU_ARCH_XTENSA BIT(QEMU_ARCH_BIT_XTENSA) >>>> +#define QEMU_ARCH_OPENRISC BIT(QEMU_ARCH_BIT_OPENRISC) >>>> +#define QEMU_ARCH_TRICORE BIT(QEMU_ARCH_BIT_TRICORE) >>>> +#define QEMU_ARCH_HPPA BIT(QEMU_ARCH_BIT_HPPA) >>>> +#define QEMU_ARCH_RISCV BIT(QEMU_ARCH_BIT_RISCV) >>>> +#define QEMU_ARCH_RX BIT(QEMU_ARCH_BIT_RX) >>>> +#define QEMU_ARCH_AVR BIT(QEMU_ARCH_BIT_AVR) >>>> +#define QEMU_ARCH_HEXAGON BIT(QEMU_ARCH_BIT_HEXAGON) >>>> +#define QEMU_ARCH_LOONGARCH BIT(QEMU_ARCH_BIT_LOONGARCH) >>>> +#define QEMU_ARCH_ALL -1 >>>> extern const uint32_t arch_type; >>> >>> What are we gaining by having a "bit" oriented enum, vs simple values >>> that can be compared too? >> >> I'm not sure what you are asking here, these definitions are heavily >> used in qemu-options.hx, and I don't plan to change them anytime soon. >> >> For the single binary I'll try to keep the command line interface with >> no change. For heterogeneous binary I plan to start with no CLI so I'm >> not really considering them. >> > > My bad, I thought we were introducing something new here. > Yes, let's stick to a minimal change. > >>> As well, it would make sense to add subvariants (aarch64, x86_64, little >>> and big endian variants for some arch), so all of them are present and >>> can be queried easily. >> >> IIUC what you are referring, this is planned for another interface, but >> not this series which is focused on introducing qemu_arch_name() and >> removing TARGET_NAME. While you don't see any improvement in duplicated >> target files as of this series, some reduction should happen in the next >> step which remove TARGET_BIG_ENDIAN uses in hw/. >> > > Yes, sounds good. It's just a bit hard to track the changes between the > cleanup of existing files, and introducing a new API to query this > information at run time. OK, in the next version I'll try to better explain where this series is going in the patch description.
On 3/4/25 16:52, Philippe Mathieu-Daudé wrote: > Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. > Use them to declare QEMU_ARCH_$target bitmasks. > > Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org> > --- > meson.build | 4 +-- > include/system/arch_init.h | 65 +++++++++++++++++++++++++------------- > system/arch_init.c | 2 +- > 3 files changed, 46 insertions(+), 25 deletions(-) If we ignore the array index in patch 10, is there any other reason for this? r~
On Wed, Mar 05, 2025 at 01:52:17AM +0100, Philippe Mathieu-Daudé wrote: > Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. > Use them to declare QEMU_ARCH_$target bitmasks. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > meson.build | 4 +-- > include/system/arch_init.h | 65 +++++++++++++++++++++++++------------- > system/arch_init.c | 2 +- > 3 files changed, 46 insertions(+), 25 deletions(-) > > diff --git a/meson.build b/meson.build > index 0a2c61d2bfa..1ab02a5d48d 100644 > --- a/meson.build > +++ b/meson.build > @@ -3357,8 +3357,8 @@ foreach target : target_dirs > config_target_data.set(k, v) > endif > endforeach > - config_target_data.set('QEMU_ARCH', > - 'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper()) > + config_target_data.set('QEMU_ARCH_BIT', > + 'QEMU_ARCH_BIT_' + config_target['TARGET_BASE_ARCH'].to_upper()) > config_target_h += {target: configure_file(output: target + '-config-target.h', > configuration: config_target_data)} > > diff --git a/include/system/arch_init.h b/include/system/arch_init.h > index d8b77440487..06e5527ec88 100644 > --- a/include/system/arch_init.h > +++ b/include/system/arch_init.h > @@ -1,29 +1,50 @@ > #ifndef QEMU_ARCH_INIT_H > #define QEMU_ARCH_INIT_H > > +#include "qemu/bitops.h" > > -enum { > - QEMU_ARCH_ALL = -1, > - QEMU_ARCH_ALPHA = (1 << 0), > - QEMU_ARCH_ARM = (1 << 1), > - QEMU_ARCH_I386 = (1 << 3), > - QEMU_ARCH_M68K = (1 << 4), > - QEMU_ARCH_MICROBLAZE = (1 << 6), > - QEMU_ARCH_MIPS = (1 << 7), > - QEMU_ARCH_PPC = (1 << 8), > - QEMU_ARCH_S390X = (1 << 9), > - QEMU_ARCH_SH4 = (1 << 10), > - QEMU_ARCH_SPARC = (1 << 11), > - QEMU_ARCH_XTENSA = (1 << 12), > - QEMU_ARCH_OPENRISC = (1 << 13), > - QEMU_ARCH_TRICORE = (1 << 16), > - QEMU_ARCH_HPPA = (1 << 18), > - QEMU_ARCH_RISCV = (1 << 19), > - QEMU_ARCH_RX = (1 << 20), > - QEMU_ARCH_AVR = (1 << 21), > - QEMU_ARCH_HEXAGON = (1 << 22), > - QEMU_ARCH_LOONGARCH = (1 << 23), > -}; > +typedef enum QemuArchBit { > + QEMU_ARCH_BIT_ALPHA = 0, > + QEMU_ARCH_BIT_ARM = 1, > + QEMU_ARCH_BIT_I386 = 3, > + QEMU_ARCH_BIT_M68K = 4, > + QEMU_ARCH_BIT_MICROBLAZE = 6, > + QEMU_ARCH_BIT_MIPS = 7, > + QEMU_ARCH_BIT_PPC = 8, > + QEMU_ARCH_BIT_S390X = 9, > + QEMU_ARCH_BIT_SH4 = 10, > + QEMU_ARCH_BIT_SPARC = 11, > + QEMU_ARCH_BIT_XTENSA = 12, > + QEMU_ARCH_BIT_OPENRISC = 13, > + QEMU_ARCH_BIT_TRICORE = 16, > + QEMU_ARCH_BIT_HPPA = 18, > + QEMU_ARCH_BIT_RISCV = 19, > + QEMU_ARCH_BIT_RX = 20, > + QEMU_ARCH_BIT_AVR = 21, > + QEMU_ARCH_BIT_HEXAGON = 22, > + QEMU_ARCH_BIT_LOONGARCH = 23, > +} QemuArchBit; I'm somewhat inclined to say we should be defining this as a QemuArch enum in QAPI, especially because that gives us the string <> int conversion that you hand-code in a latter patch. With regards, Daniel
On Wed, Mar 05, 2025 at 08:55:58AM +0000, Daniel P. Berrangé wrote: > On Wed, Mar 05, 2025 at 01:52:17AM +0100, Philippe Mathieu-Daudé wrote: > > Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. > > Use them to declare QEMU_ARCH_$target bitmasks. > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > --- > > meson.build | 4 +-- > > include/system/arch_init.h | 65 +++++++++++++++++++++++++------------- > > system/arch_init.c | 2 +- > > 3 files changed, 46 insertions(+), 25 deletions(-) > > > > diff --git a/meson.build b/meson.build > > index 0a2c61d2bfa..1ab02a5d48d 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -3357,8 +3357,8 @@ foreach target : target_dirs > > config_target_data.set(k, v) > > endif > > endforeach > > - config_target_data.set('QEMU_ARCH', > > - 'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper()) > > + config_target_data.set('QEMU_ARCH_BIT', > > + 'QEMU_ARCH_BIT_' + config_target['TARGET_BASE_ARCH'].to_upper()) > > config_target_h += {target: configure_file(output: target + '-config-target.h', > > configuration: config_target_data)} > > > > diff --git a/include/system/arch_init.h b/include/system/arch_init.h > > index d8b77440487..06e5527ec88 100644 > > --- a/include/system/arch_init.h > > +++ b/include/system/arch_init.h > > @@ -1,29 +1,50 @@ > > #ifndef QEMU_ARCH_INIT_H > > #define QEMU_ARCH_INIT_H > > > > +#include "qemu/bitops.h" > > > > -enum { > > - QEMU_ARCH_ALL = -1, > > - QEMU_ARCH_ALPHA = (1 << 0), > > - QEMU_ARCH_ARM = (1 << 1), > > - QEMU_ARCH_I386 = (1 << 3), > > - QEMU_ARCH_M68K = (1 << 4), > > - QEMU_ARCH_MICROBLAZE = (1 << 6), > > - QEMU_ARCH_MIPS = (1 << 7), > > - QEMU_ARCH_PPC = (1 << 8), > > - QEMU_ARCH_S390X = (1 << 9), > > - QEMU_ARCH_SH4 = (1 << 10), > > - QEMU_ARCH_SPARC = (1 << 11), > > - QEMU_ARCH_XTENSA = (1 << 12), > > - QEMU_ARCH_OPENRISC = (1 << 13), > > - QEMU_ARCH_TRICORE = (1 << 16), > > - QEMU_ARCH_HPPA = (1 << 18), > > - QEMU_ARCH_RISCV = (1 << 19), > > - QEMU_ARCH_RX = (1 << 20), > > - QEMU_ARCH_AVR = (1 << 21), > > - QEMU_ARCH_HEXAGON = (1 << 22), > > - QEMU_ARCH_LOONGARCH = (1 << 23), > > -}; > > +typedef enum QemuArchBit { > > + QEMU_ARCH_BIT_ALPHA = 0, > > + QEMU_ARCH_BIT_ARM = 1, > > + QEMU_ARCH_BIT_I386 = 3, > > + QEMU_ARCH_BIT_M68K = 4, > > + QEMU_ARCH_BIT_MICROBLAZE = 6, > > + QEMU_ARCH_BIT_MIPS = 7, > > + QEMU_ARCH_BIT_PPC = 8, > > + QEMU_ARCH_BIT_S390X = 9, > > + QEMU_ARCH_BIT_SH4 = 10, > > + QEMU_ARCH_BIT_SPARC = 11, > > + QEMU_ARCH_BIT_XTENSA = 12, > > + QEMU_ARCH_BIT_OPENRISC = 13, > > + QEMU_ARCH_BIT_TRICORE = 16, > > + QEMU_ARCH_BIT_HPPA = 18, > > + QEMU_ARCH_BIT_RISCV = 19, > > + QEMU_ARCH_BIT_RX = 20, > > + QEMU_ARCH_BIT_AVR = 21, > > + QEMU_ARCH_BIT_HEXAGON = 22, > > + QEMU_ARCH_BIT_LOONGARCH = 23, > > +} QemuArchBit; > > I'm somewhat inclined to say we should be defining this as a QemuArch > enum in QAPI, especially because that gives us the string <> int > conversion that you hand-code in a latter patch. Having said that, the auto-generated string/int conversion won't work if we have differing name mappings based on endian/target bits size. So scratch that idea. With regards, Daniel
diff --git a/meson.build b/meson.build index 0a2c61d2bfa..1ab02a5d48d 100644 --- a/meson.build +++ b/meson.build @@ -3357,8 +3357,8 @@ foreach target : target_dirs config_target_data.set(k, v) endif endforeach - config_target_data.set('QEMU_ARCH', - 'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper()) + config_target_data.set('QEMU_ARCH_BIT', + 'QEMU_ARCH_BIT_' + config_target['TARGET_BASE_ARCH'].to_upper()) config_target_h += {target: configure_file(output: target + '-config-target.h', configuration: config_target_data)} diff --git a/include/system/arch_init.h b/include/system/arch_init.h index d8b77440487..06e5527ec88 100644 --- a/include/system/arch_init.h +++ b/include/system/arch_init.h @@ -1,29 +1,50 @@ #ifndef QEMU_ARCH_INIT_H #define QEMU_ARCH_INIT_H +#include "qemu/bitops.h" -enum { - QEMU_ARCH_ALL = -1, - QEMU_ARCH_ALPHA = (1 << 0), - QEMU_ARCH_ARM = (1 << 1), - QEMU_ARCH_I386 = (1 << 3), - QEMU_ARCH_M68K = (1 << 4), - QEMU_ARCH_MICROBLAZE = (1 << 6), - QEMU_ARCH_MIPS = (1 << 7), - QEMU_ARCH_PPC = (1 << 8), - QEMU_ARCH_S390X = (1 << 9), - QEMU_ARCH_SH4 = (1 << 10), - QEMU_ARCH_SPARC = (1 << 11), - QEMU_ARCH_XTENSA = (1 << 12), - QEMU_ARCH_OPENRISC = (1 << 13), - QEMU_ARCH_TRICORE = (1 << 16), - QEMU_ARCH_HPPA = (1 << 18), - QEMU_ARCH_RISCV = (1 << 19), - QEMU_ARCH_RX = (1 << 20), - QEMU_ARCH_AVR = (1 << 21), - QEMU_ARCH_HEXAGON = (1 << 22), - QEMU_ARCH_LOONGARCH = (1 << 23), -}; +typedef enum QemuArchBit { + QEMU_ARCH_BIT_ALPHA = 0, + QEMU_ARCH_BIT_ARM = 1, + QEMU_ARCH_BIT_I386 = 3, + QEMU_ARCH_BIT_M68K = 4, + QEMU_ARCH_BIT_MICROBLAZE = 6, + QEMU_ARCH_BIT_MIPS = 7, + QEMU_ARCH_BIT_PPC = 8, + QEMU_ARCH_BIT_S390X = 9, + QEMU_ARCH_BIT_SH4 = 10, + QEMU_ARCH_BIT_SPARC = 11, + QEMU_ARCH_BIT_XTENSA = 12, + QEMU_ARCH_BIT_OPENRISC = 13, + QEMU_ARCH_BIT_TRICORE = 16, + QEMU_ARCH_BIT_HPPA = 18, + QEMU_ARCH_BIT_RISCV = 19, + QEMU_ARCH_BIT_RX = 20, + QEMU_ARCH_BIT_AVR = 21, + QEMU_ARCH_BIT_HEXAGON = 22, + QEMU_ARCH_BIT_LOONGARCH = 23, +} QemuArchBit; + +#define QEMU_ARCH_ALPHA BIT(QEMU_ARCH_BIT_ALPHA) +#define QEMU_ARCH_ARM BIT(QEMU_ARCH_BIT_ARM) +#define QEMU_ARCH_I386 BIT(QEMU_ARCH_BIT_I386) +#define QEMU_ARCH_M68K BIT(QEMU_ARCH_BIT_M68K) +#define QEMU_ARCH_MICROBLAZE BIT(QEMU_ARCH_BIT_MICROBLAZE) +#define QEMU_ARCH_MIPS BIT(QEMU_ARCH_BIT_MIPS) +#define QEMU_ARCH_PPC BIT(QEMU_ARCH_BIT_PPC) +#define QEMU_ARCH_S390X BIT(QEMU_ARCH_BIT_S390X) +#define QEMU_ARCH_SH4 BIT(QEMU_ARCH_BIT_SH4) +#define QEMU_ARCH_SPARC BIT(QEMU_ARCH_BIT_SPARC) +#define QEMU_ARCH_XTENSA BIT(QEMU_ARCH_BIT_XTENSA) +#define QEMU_ARCH_OPENRISC BIT(QEMU_ARCH_BIT_OPENRISC) +#define QEMU_ARCH_TRICORE BIT(QEMU_ARCH_BIT_TRICORE) +#define QEMU_ARCH_HPPA BIT(QEMU_ARCH_BIT_HPPA) +#define QEMU_ARCH_RISCV BIT(QEMU_ARCH_BIT_RISCV) +#define QEMU_ARCH_RX BIT(QEMU_ARCH_BIT_RX) +#define QEMU_ARCH_AVR BIT(QEMU_ARCH_BIT_AVR) +#define QEMU_ARCH_HEXAGON BIT(QEMU_ARCH_BIT_HEXAGON) +#define QEMU_ARCH_LOONGARCH BIT(QEMU_ARCH_BIT_LOONGARCH) +#define QEMU_ARCH_ALL -1 extern const uint32_t arch_type; diff --git a/system/arch_init.c b/system/arch_init.c index b9147af93cb..fedbb18e2cc 100644 --- a/system/arch_init.c +++ b/system/arch_init.c @@ -24,4 +24,4 @@ #include "qemu/osdep.h" #include "system/arch_init.h" -const uint32_t arch_type = QEMU_ARCH; +const uint32_t arch_type = BIT(QEMU_ARCH_BIT);
Declare QEMU_ARCH_BIT_$target as QemuArchBit enum. Use them to declare QEMU_ARCH_$target bitmasks. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- meson.build | 4 +-- include/system/arch_init.h | 65 +++++++++++++++++++++++++------------- system/arch_init.c | 2 +- 3 files changed, 46 insertions(+), 25 deletions(-)