| Message ID | 20231212123401.37493-16-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | exec: Rework of various headers (user focused) | expand |
On 12/12/23, Philippe Mathieu-Daudé wrote: > We usually check target endianess before swapping values, > so target_words_bigendian() declaration makes sense in > "exec/tswap.h" with the target swapping helpers. > > Remove "hw/core/cpu.h" when it was only included to get > the target_words_bigendian() declaration. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/exec/tswap.h | 12 +++++++++++- > include/hw/core/cpu.h | 11 ----------- > cpu-target.c | 1 + > disas/disas.c | 1 + > hw/audio/virtio-snd.c | 2 +- > hw/core/cpu-sysemu.c | 2 +- > hw/core/generic-loader.c | 2 +- > hw/display/vga.c | 2 +- > hw/virtio/virtio.c | 1 + > 9 files changed, 18 insertions(+), 16 deletions(-) > > diff --git a/include/exec/tswap.h b/include/exec/tswap.h > index 68944a880b..77954bbc2b 100644 > --- a/include/exec/tswap.h > +++ b/include/exec/tswap.h > @@ -8,9 +8,19 @@ > #ifndef TSWAP_H > #define TSWAP_H > > -#include "hw/core/cpu.h" > #include "qemu/bswap.h" This breaks system/qtest.c which gets first_cpu from hw/core/cpu.h through the inclusion of tswap.h... ../system/qtest.c:548:33: error: ‘first_cpu’ undeclared (first use in this function); 548 | address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, | ^~~~~~~~~ Adding hw/core/cpu.h to system/qtest.c seems sufficient. Otherwise, Reviewed-by: Anton Johansson <anjo@rev.ng>
On 13/12/23 12:20, Anton Johansson wrote: > On 12/12/23, Philippe Mathieu-Daudé wrote: >> We usually check target endianess before swapping values, >> so target_words_bigendian() declaration makes sense in >> "exec/tswap.h" with the target swapping helpers. >> >> Remove "hw/core/cpu.h" when it was only included to get >> the target_words_bigendian() declaration. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/exec/tswap.h | 12 +++++++++++- >> include/hw/core/cpu.h | 11 ----------- >> cpu-target.c | 1 + >> disas/disas.c | 1 + >> hw/audio/virtio-snd.c | 2 +- >> hw/core/cpu-sysemu.c | 2 +- >> hw/core/generic-loader.c | 2 +- >> hw/display/vga.c | 2 +- >> hw/virtio/virtio.c | 1 + >> 9 files changed, 18 insertions(+), 16 deletions(-) >> >> diff --git a/include/exec/tswap.h b/include/exec/tswap.h >> index 68944a880b..77954bbc2b 100644 >> --- a/include/exec/tswap.h >> +++ b/include/exec/tswap.h >> @@ -8,9 +8,19 @@ >> #ifndef TSWAP_H >> #define TSWAP_H >> >> -#include "hw/core/cpu.h" >> #include "qemu/bswap.h" > > This breaks system/qtest.c which gets first_cpu from hw/core/cpu.h > through the inclusion of tswap.h... > > ../system/qtest.c:548:33: error: ‘first_cpu’ undeclared (first use in this function); > 548 | address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, > | ^~~~~~~~~ Doh sorry, in this series I dropped some already reviewed patches, and extracted some not-that-related series. I was lazy to add the many 'Based-on:' tags on the cover. What you mentioned here is fixed here: https://lore.kernel.org/qemu-devel/20231212113016.29808-1-philmd@linaro.org/ > Adding hw/core/cpu.h to system/qtest.c seems sufficient. > > Otherwise, > Reviewed-by: Anton Johansson <anjo@rev.ng> Thanks!
diff --git a/include/exec/tswap.h b/include/exec/tswap.h index 68944a880b..77954bbc2b 100644 --- a/include/exec/tswap.h +++ b/include/exec/tswap.h @@ -8,9 +8,19 @@ #ifndef TSWAP_H #define TSWAP_H -#include "hw/core/cpu.h" #include "qemu/bswap.h" +/** + * target_words_bigendian: + * Returns true if the (default) endianness of the target is big endian, + * false otherwise. Note that in target-specific code, you can use + * TARGET_BIG_ENDIAN directly instead. On the other hand, common + * code should normally never need to know about the endianness of the + * target, so please do *not* use this function unless you know very well + * what you are doing! + */ +bool target_words_bigendian(void); + /* * If we're in target-specific code, we can hard-code the swapping * condition, otherwise we have to do (slower) run-time checks. diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 3208b938f7..d6b077da71 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1153,17 +1153,6 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp); void cpu_exec_unrealizefn(CPUState *cpu); void cpu_exec_reset_hold(CPUState *cpu); -/** - * target_words_bigendian: - * Returns true if the (default) endianness of the target is big endian, - * false otherwise. Note that in target-specific code, you can use - * TARGET_BIG_ENDIAN directly instead. On the other hand, common - * code should normally never need to know about the endianness of the - * target, so please do *not* use this function unless you know very well - * what you are doing! - */ -bool target_words_bigendian(void); - const char *target_name(void); void page_size_init(void); diff --git a/cpu-target.c b/cpu-target.c index 997ca2e846..4c1e19ca81 100644 --- a/cpu-target.c +++ b/cpu-target.c @@ -33,6 +33,7 @@ #endif #include "sysemu/cpus.h" #include "sysemu/tcg.h" +#include "exec/tswap.h" #include "exec/replay-core.h" #include "exec/cpu-common.h" #include "exec/exec-all.h" diff --git a/disas/disas.c b/disas/disas.c index 0d2d06c2ec..89a1e7d767 100644 --- a/disas/disas.c +++ b/disas/disas.c @@ -6,6 +6,7 @@ #include "disas/disas.h" #include "disas/capstone.h" #include "hw/core/cpu.h" +#include "exec/tswap.h" #include "exec/memory.h" /* Filled in by elfload.c. Simplistic, but will do for now. */ diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c index 137fa77a01..a80f23aa71 100644 --- a/hw/audio/virtio-snd.c +++ b/hw/audio/virtio-snd.c @@ -20,11 +20,11 @@ #include "qemu/log.h" #include "qemu/error-report.h" #include "include/qemu/lockable.h" +#include "exec/tswap.h" #include "sysemu/runstate.h" #include "trace.h" #include "qapi/error.h" #include "hw/audio/virtio-snd.h" -#include "hw/core/cpu.h" #define VIRTIO_SOUND_VM_VERSION 1 #define VIRTIO_SOUND_JACK_DEFAULT 0 diff --git a/hw/core/cpu-sysemu.c b/hw/core/cpu-sysemu.c index d0d6a910f9..2a9a2a4eb5 100644 --- a/hw/core/cpu-sysemu.c +++ b/hw/core/cpu-sysemu.c @@ -20,7 +20,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" -#include "hw/core/cpu.h" +#include "exec/tswap.h" #include "hw/core/sysemu-cpu-ops.h" bool cpu_paging_enabled(const CPUState *cpu) diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c index d4b5c501d8..ea8628b892 100644 --- a/hw/core/generic-loader.c +++ b/hw/core/generic-loader.c @@ -31,7 +31,7 @@ */ #include "qemu/osdep.h" -#include "hw/core/cpu.h" +#include "exec/tswap.h" #include "sysemu/dma.h" #include "sysemu/reset.h" #include "hw/boards.h" diff --git a/hw/display/vga.c b/hw/display/vga.c index 37557c3442..d11a30a785 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -26,7 +26,7 @@ #include "qemu/units.h" #include "sysemu/reset.h" #include "qapi/error.h" -#include "hw/core/cpu.h" +#include "exec/tswap.h" #include "hw/display/vga.h" #include "hw/i386/x86.h" #include "hw/pci/pci.h" diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 3a160f86ed..28dc1f4c15 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -20,6 +20,7 @@ #include "qemu/log.h" #include "qemu/main-loop.h" #include "qemu/module.h" +#include "exec/tswap.h" #include "qom/object_interfaces.h" #include "hw/core/cpu.h" #include "hw/virtio/virtio.h"
We usually check target endianess before swapping values, so target_words_bigendian() declaration makes sense in "exec/tswap.h" with the target swapping helpers. Remove "hw/core/cpu.h" when it was only included to get the target_words_bigendian() declaration. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/exec/tswap.h | 12 +++++++++++- include/hw/core/cpu.h | 11 ----------- cpu-target.c | 1 + disas/disas.c | 1 + hw/audio/virtio-snd.c | 2 +- hw/core/cpu-sysemu.c | 2 +- hw/core/generic-loader.c | 2 +- hw/display/vga.c | 2 +- hw/virtio/virtio.c | 1 + 9 files changed, 18 insertions(+), 16 deletions(-)