From af1bb59c07c889c59cc22322d6eccb678991a299 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 16 Apr 2021 14:55:38 +0100 Subject: [PATCH 1/7] osdep: include glib-compat.h before other QEMU headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit glib-compat.h is sort of like a system header, and it needs to include system headers (glib.h) that may dislike being included under 'extern "C"'. Move it right after all system headers and before all other QEMU headers. Signed-off-by: Paolo Bonzini Reviewed-by: Daniel P. Berrangé Reviewed-by: Peter Maydell Acked-by: Paolo Bonzini Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20210416135543.20382-2-peter.maydell@linaro.org [PMM: Added comment about why glib-compat.h is special] Signed-off-by: Peter Maydell --- include/qemu/osdep.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index ba15be9c56..ab84ecc7c1 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -111,6 +111,13 @@ extern int daemon(int, int); #define WEXITSTATUS(x) (x) #endif +/* + * This is somewhat like a system header; it must be outside any extern "C" + * block because it includes system headers itself, including glib.h, + * which will not compile if inside an extern "C" block. + */ +#include "glib-compat.h" + #ifdef _WIN32 #include "sysemu/os-win32.h" #endif @@ -123,7 +130,6 @@ extern int daemon(int, int); #include #endif -#include "glib-compat.h" #include "qemu/typedefs.h" /* From 875df03b221ef6a67f175f1f283ea5598f717da5 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 16 Apr 2021 14:55:39 +0100 Subject: [PATCH 2/7] osdep: protect qemu/osdep.h with extern "C" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit System headers may include templates if compiled with a C++ compiler, which cause the compiler to complain if qemu/osdep.h is included within a C++ source file's 'extern "C"' block. Add an 'extern "C"' block directly to qemu/osdep.h, so that system headers can be kept out of it. There is a stray declaration early in qemu/osdep.h, which needs to be special cased. Add a definition in qemu/compiler.h to make it look nice. config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h are included outside the 'extern "C"' block; that is not an issue because they consist entirely of preprocessor directives. This allows us to move the include of osdep.h in our two C++ source files outside the extern "C" block they were previously using for it, which in turn means that they compile successfully against newer versions of glib which insist that glib.h is *not* inside an extern "C" block. Signed-off-by: Paolo Bonzini Reviewed-by: Daniel P. Berrangé Acked-by: Paolo Bonzini Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20210416135543.20382-3-peter.maydell@linaro.org [PMM: Moved disas/arm-a64.cc osdep.h include out of its extern "C" block; explained in commit message why we're doing this] Signed-off-by: Peter Maydell --- disas/arm-a64.cc | 2 +- disas/nanomips.cpp | 2 +- include/qemu/compiler.h | 6 ++++++ include/qemu/osdep.h | 10 +++++++++- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/disas/arm-a64.cc b/disas/arm-a64.cc index 9fa779e175..27613d4b25 100644 --- a/disas/arm-a64.cc +++ b/disas/arm-a64.cc @@ -17,8 +17,8 @@ * along with this program. If not, see . */ -extern "C" { #include "qemu/osdep.h" +extern "C" { #include "disas/dis-asm.h" } diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp index 2b09655271..8ddef897f0 100644 --- a/disas/nanomips.cpp +++ b/disas/nanomips.cpp @@ -27,8 +27,8 @@ * Reference Manual", Revision 01.01, April 27, 2018 */ -extern "C" { #include "qemu/osdep.h" +extern "C" { #include "disas/dis-asm.h" } diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index cf28bb2bcd..091c45248b 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -11,6 +11,12 @@ #define QEMU_STATIC_ANALYSIS 1 #endif +#ifdef __cplusplus +#define QEMU_EXTERN_C extern "C" +#else +#define QEMU_EXTERN_C extern +#endif + #define QEMU_NORETURN __attribute__ ((__noreturn__)) #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index ab84ecc7c1..6d8562eaf4 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -57,7 +57,7 @@ #define daemon qemu_fake_daemon_function #include #undef daemon -extern int daemon(int, int); +QEMU_EXTERN_C int daemon(int, int); #endif #ifdef _WIN32 @@ -118,6 +118,10 @@ extern int daemon(int, int); */ #include "glib-compat.h" +#ifdef __cplusplus +extern "C" { +#endif + #ifdef _WIN32 #include "sysemu/os-win32.h" #endif @@ -728,4 +732,8 @@ static inline int platform_does_not_support_system(const char *command) } #endif /* !HAVE_SYSTEM_FUNCTION */ +#ifdef __cplusplus +} +#endif + #endif From ec63ca2d356f7be018138d7f66b13dffd2396b9f Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 16 Apr 2021 14:55:40 +0100 Subject: [PATCH 3/7] include/qemu/osdep.h: Move system includes to top MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mostly osdep.h puts the system includes at the top of the file; but there are a couple of exceptions where we include a system header halfway through the file. Move these up to the top with the rest so that all the system headers we include are included before we include os-win32.h or os-posix.h. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Daniel P. Berrangé Acked-by: Paolo Bonzini Reviewed-by: Richard Henderson Message-id: 20210416135543.20382-4-peter.maydell@linaro.org Message-id: 20210414184343.26235-1-peter.maydell@linaro.org --- include/qemu/osdep.h | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 6d8562eaf4..cb2a07e472 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -104,6 +104,15 @@ QEMU_EXTERN_C int daemon(int, int); #include #include +#ifdef CONFIG_IOVEC +#include +#endif + +#if defined(__linux__) && defined(__sparc__) +/* The SPARC definition of QEMU_VMALLOC_ALIGN needs SHMLBA */ +#include +#endif + #ifndef _WIN32 #include #else @@ -111,6 +120,10 @@ QEMU_EXTERN_C int daemon(int, int); #define WEXITSTATUS(x) (x) #endif +#ifdef __APPLE__ +#include +#endif + /* * This is somewhat like a system header; it must be outside any extern "C" * block because it includes system headers itself, including glib.h, @@ -130,10 +143,6 @@ extern "C" { #include "sysemu/os-posix.h" #endif -#ifdef __APPLE__ -#include -#endif - #include "qemu/typedefs.h" /* @@ -469,7 +478,6 @@ void qemu_anon_ram_free(void *ptr, size_t size); /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */ # define QEMU_VMALLOC_ALIGN (256 * 4096) #elif defined(__linux__) && defined(__sparc__) -#include # define QEMU_VMALLOC_ALIGN MAX(qemu_real_host_page_size, SHMLBA) #else # define QEMU_VMALLOC_ALIGN qemu_real_host_page_size @@ -549,8 +557,6 @@ struct iovec { ssize_t readv(int fd, const struct iovec *iov, int iov_cnt); ssize_t writev(int fd, const struct iovec *iov, int iov_cnt); -#else -#include #endif #ifdef _WIN32 From 1df0878cff267128393b0076da4a96467ac04094 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 15 Apr 2021 19:23:53 +0100 Subject: [PATCH 4/7] hw/arm/armsse: Give SSE-300 its own Property array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SSE-300 currently shares the SSE-200 Property array. This is bad principally because the default values of the CPU0_FPU and CPU0_DSP properties disable the FPU and DSP on the CPU. That is correct for the SSE-200 but not the SSE-300. Give the SSE-300 its own Property array with the correct SSE-300 specific settings: * SSE-300 has only one CPU, so no CPU1* properties * SSE-300 CPU has FPU and DSP Buglink: https://bugs.launchpad.net/qemu/+bug/1923861 Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-id: 20210415182353.8173-1-peter.maydell@linaro.org --- hw/arm/armsse.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index e5aeb9e485..170dea8632 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -84,7 +84,7 @@ static Property iotkit_properties[] = { DEFINE_PROP_END_OF_LIST() }; -static Property armsse_properties[] = { +static Property sse200_properties[] = { DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, MemoryRegion *), DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), @@ -97,6 +97,17 @@ static Property armsse_properties[] = { DEFINE_PROP_END_OF_LIST() }; +static Property sse300_properties[] = { + DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, + MemoryRegion *), + DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), + DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), + DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), + DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true), + DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], true), + DEFINE_PROP_END_OF_LIST() +}; + static const ARMSSEDeviceInfo iotkit_devices[] = { { .name = "timer0", @@ -519,7 +530,7 @@ static const ARMSSEInfo armsse_variants[] = { .has_cpuid = true, .has_cpu_pwrctrl = false, .has_sse_counter = false, - .props = armsse_properties, + .props = sse200_properties, .devinfo = sse200_devices, .irq_is_common = sse200_irq_is_common, }, @@ -537,7 +548,7 @@ static const ARMSSEInfo armsse_variants[] = { .has_cpuid = true, .has_cpu_pwrctrl = true, .has_sse_counter = true, - .props = armsse_properties, + .props = sse300_properties, .devinfo = sse300_devices, .irq_is_common = sse300_irq_is_common, }, From 330ef14e6e749919c5c70e9fd6f73aaeac8a15ae Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 16 Apr 2021 11:40:10 +0100 Subject: [PATCH 5/7] hw/arm/armsse: Make SSE-300 use Cortex-M55 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSE-300 has a Cortex-M55 (which was the whole reason for us modelling it), but we forgot to actually update the code to let it have a different CPU type from the IoTKit and SSE-200. Add CPU type as a field for ARMSSEInfo instead of hardcoding it to always use a Cortex-M33. Buglink: https://bugs.launchpad.net/qemu/+bug/1923861 Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-id: 20210416104010.13228-1-peter.maydell@linaro.org --- hw/arm/armsse.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index 170dea8632..2e5d0679e7 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -56,6 +56,7 @@ typedef struct ARMSSEDeviceInfo { struct ARMSSEInfo { const char *name; + const char *cpu_type; uint32_t sse_version; int sram_banks; int num_cpus; @@ -501,6 +502,7 @@ static const ARMSSEInfo armsse_variants[] = { { .name = TYPE_IOTKIT, .sse_version = ARMSSE_IOTKIT, + .cpu_type = ARM_CPU_TYPE_NAME("cortex-m33"), .sram_banks = 1, .num_cpus = 1, .sys_version = 0x41743, @@ -519,6 +521,7 @@ static const ARMSSEInfo armsse_variants[] = { { .name = TYPE_SSE200, .sse_version = ARMSSE_SSE200, + .cpu_type = ARM_CPU_TYPE_NAME("cortex-m33"), .sram_banks = 4, .num_cpus = 2, .sys_version = 0x22041743, @@ -537,6 +540,7 @@ static const ARMSSEInfo armsse_variants[] = { { .name = TYPE_SSE300, .sse_version = ARMSSE_SSE300, + .cpu_type = ARM_CPU_TYPE_NAME("cortex-m55"), .sram_banks = 2, .num_cpus = 1, .sys_version = 0x7e00043b, @@ -719,8 +723,7 @@ static void armsse_init(Object *obj) name = g_strdup_printf("armv7m%d", i); object_initialize_child(OBJECT(&s->cluster[i]), name, &s->armv7m[i], TYPE_ARMV7M); - qdev_prop_set_string(DEVICE(&s->armv7m[i]), "cpu-type", - ARM_CPU_TYPE_NAME("cortex-m33")); + qdev_prop_set_string(DEVICE(&s->armv7m[i]), "cpu-type", info->cpu_type); g_free(name); name = g_strdup_printf("arm-sse-cpu-container%d", i); memory_region_init(&s->cpu_container[i], obj, name, UINT64_MAX); From c57b27ea89ac3ca8a4bc6b682231823f081478d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 16 Apr 2021 18:02:07 +0100 Subject: [PATCH 6/7] target/arm: drop CF_LAST_IO/dc->condjump check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a left over erroneous check from the days front-ends handled io start/end themselves. Regardless just because IO could be performed on the last instruction doesn't obligate the front end to do so. This fixes an abort faced by the aspeed execute-in-place support which will necessarily trigger this state (even before the one-shot CF_LAST_IO fix). The test still seems to hang once it attempts to boot the Linux kernel but I suspect this is an unrelated issue with icount and the timer handling code. The original intention of the cpu_abort (added in commit 2e70f6efa8b9 when the icount stuff was first added) seems to have been to act as an assert() to catch an unhandled corner case where the generated code would be something like: conditional branch to condlabel if its cc failed implementation of the insn (a conditional branch or trap) code emitted by gen_io_end() condlabel: gen_goto_tb or equivalent thing to go to next insn At runtime the cc-failed case would skip over the code emitted by gen_io_end(), leaving the can_do_io flag incorrectly set. In commit ba3e7926691ed33 we switched to an implementation which always clears can_do_io at the start of the following TB instead of trying to clear it at the end of a TB that did IO. So the corner case that this cpu_abort() was trying to flag is no longer possible, because the gen_io_end() call has been deleted. We can therefore safely remove the no-longer-valid assertion. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Peter Maydell Message-id: 20210416170207.12504-1-alex.bennee@linaro.org Cc: Cédric Le Goater Signed-off-by: Peter Maydell --- target/arm/translate.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 62b1c2081b..7103da2d7a 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -9199,11 +9199,6 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); - if (tb_cflags(dc->base.tb) & CF_LAST_IO && dc->condjmp) { - /* FIXME: This can theoretically happen with self-modifying code. */ - cpu_abort(cpu, "IO on conditional branch instruction"); - } - /* At this stage dc->condjmp will only be set when the skipped instruction was a conditional branch or trap, and the PC has already been written. */ From 277aed998ac2cd3649bf0e13b22f47769519eb61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 15 Apr 2021 17:24:53 +0100 Subject: [PATCH 7/7] accel/tcg: avoid re-translating one-shot instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By definition a single instruction is capable of being an IO instruction. This avoids a problem of triggering a cpu_io_recompile on a non-recorded translation which then fails because it expects tcg_tb_lookup() to succeed unconditionally. The normal use case requires a TB to be able to resolve machine state. The other users of tcg_tb_lookup() are able to tolerate a missing TB if the machine state has been resolved by other means - which in the single-shot case is always true because machine state is synced at the start of a block. Reported-by: Peter Maydell Signed-off-by: Alex Bennée Reviewed-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20210415162454.22056-1-alex.bennee@linaro.org Signed-off-by: Peter Maydell --- accel/tcg/translate-all.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index ba6ab09790..b12d0898d0 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, if (phys_pc == -1) { /* Generate a one-shot TB with 1 insn in it */ - cflags = (cflags & ~CF_COUNT_MASK) | 1; + cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1; } max_insns = cflags & CF_COUNT_MASK;