[PULL 0/7] queue of proposed rc4 fixes

[PULL 0/7] queue of proposed rc4 fixes

[Peter Maydell]

This pullreq contains fixes for the remaining "not fixed yet" issues
in the 6.0 Planning page:
 * Fix compile failures of C++ files with new glib headers
 * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
 * accel/tcg: Fix assertion failure executing from non-RAM with -icount

None of these are 100% rc4-worthy on their own, but taken all together
I think they justify rolling another release candidate.

thanks
-- PMM

The following changes since commit 8fe9f1f891eff4e37f82622b7480ee748bf4af74:

  Update version for v6.0.0-rc3 release (2021-04-14 22:06:18 +0100)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210417

for you to fetch changes up to 277aed998ac2cd3649bf0e13b22f47769519eb61:

  accel/tcg: avoid re-translating one-shot instructions (2021-04-17 18:51:14 +0100)

----------------------------------------------------------------
Fixes for rc4:
 * Fix compile failures of C++ files with new glib headers
 * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
 * accel/tcg: Fix assertion failure executing from non-RAM with -icount

----------------------------------------------------------------
Alex Bennée (2):
      target/arm: drop CF_LAST_IO/dc->condjump check
      accel/tcg: avoid re-translating one-shot instructions

Paolo Bonzini (2):
      osdep: include glib-compat.h before other QEMU headers
      osdep: protect qemu/osdep.h with extern "C"

Peter Maydell (3):
      include/qemu/osdep.h: Move system includes to top
      hw/arm/armsse: Give SSE-300 its own Property array
      hw/arm/armsse: Make SSE-300 use Cortex-M55

 include/qemu/compiler.h   |  6 ++++++
 include/qemu/osdep.h      | 38 +++++++++++++++++++++++++++++---------
 accel/tcg/translate-all.c |  2 +-
 hw/arm/armsse.c           | 24 +++++++++++++++++++-----
 target/arm/translate.c    |  5 -----
 disas/arm-a64.cc          |  2 +-
 disas/nanomips.cpp        |  2 +-
 7 files changed, 57 insertions(+), 22 deletions(-)

[PULL 1/7] osdep: include glib-compat.h before other QEMU headers

[Peter Maydell]

From: Paolo Bonzini <pbonzini@redhat.com>

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 <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210416135543.20382-2-peter.maydell@linaro.org
[PMM: Added comment about why glib-compat.h is special]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 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 ba15be9c569..ab84ecc7c1c 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 <AvailabilityMacros.h>
 #endif
 
-#include "glib-compat.h"
 #include "qemu/typedefs.h"
 
 /*
-- 
2.20.1

[PULL 2/7] osdep: protect qemu/osdep.h with extern "C"

[Peter Maydell]

From: Paolo Bonzini <pbonzini@redhat.com>

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 <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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 <peter.maydell@linaro.org>
---
 include/qemu/compiler.h |  6 ++++++
 include/qemu/osdep.h    | 10 +++++++++-
 disas/arm-a64.cc        |  2 +-
 disas/nanomips.cpp      |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index cf28bb2bcd7..091c45248b0 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 ab84ecc7c1c..6d8562eaf40 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -57,7 +57,7 @@
 #define daemon qemu_fake_daemon_function
 #include <stdlib.h>
 #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
diff --git a/disas/arm-a64.cc b/disas/arm-a64.cc
index 9fa779e175e..27613d4b256 100644
--- a/disas/arm-a64.cc
+++ b/disas/arm-a64.cc
@@ -17,8 +17,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-extern "C" {
 #include "qemu/osdep.h"
+extern "C" {
 #include "disas/dis-asm.h"
 }
 
diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 2b096552719..8ddef897f0d 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"
 }
 
-- 
2.20.1

[PULL 3/7] include/qemu/osdep.h: Move system includes to top

[Peter Maydell]

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 <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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 6d8562eaf40..cb2a07e472e 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -104,6 +104,15 @@ QEMU_EXTERN_C int daemon(int, int);
 #include <setjmp.h>
 #include <signal.h>
 
+#ifdef CONFIG_IOVEC
+#include <sys/uio.h>
+#endif
+
+#if defined(__linux__) && defined(__sparc__)
+/* The SPARC definition of QEMU_VMALLOC_ALIGN needs SHMLBA */
+#include <sys/shm.h>
+#endif
+
 #ifndef _WIN32
 #include <sys/wait.h>
 #else
@@ -111,6 +120,10 @@ QEMU_EXTERN_C int daemon(int, int);
 #define WEXITSTATUS(x) (x)
 #endif
 
+#ifdef __APPLE__
+#include <AvailabilityMacros.h>
+#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 <AvailabilityMacros.h>
-#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 <sys/shm.h>
 #  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 <sys/uio.h>
 #endif
 
 #ifdef _WIN32
-- 
2.20.1

[PULL 4/7] hw/arm/armsse: Give SSE-300 its own Property array

[Peter Maydell]

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 <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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 e5aeb9e485f..170dea8632d 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,
     },
-- 
2.20.1

[PULL 5/7] hw/arm/armsse: Make SSE-300 use Cortex-M55

[Peter Maydell]

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 <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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 170dea8632d..2e5d0679e7b 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);
-- 
2.20.1

[PULL 6/7] target/arm: drop CF_LAST_IO/dc->condjump check

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

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 <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20210416170207.12504-1-alex.bennee@linaro.org
Cc: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/translate.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 62b1c2081b6..7103da2d7ab 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.  */
-- 
2.20.1

[PULL 7/7] accel/tcg: avoid re-translating one-shot instructions

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

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 <peter.maydell@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210415162454.22056-1-alex.bennee@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 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 ba6ab09790e..b12d0898d0a 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;
-- 
2.20.1

Re: [PULL 0/7] queue of proposed rc4 fixes

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210417194205.17057-1-peter.maydell@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210417194205.17057-1-peter.maydell@linaro.org
Subject: [PULL 0/7] queue of proposed rc4 fixes

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210417194205.17057-1-peter.maydell@linaro.org -> patchew/20210417194205.17057-1-peter.maydell@linaro.org
Switched to a new branch 'test'
72e612f accel/tcg: avoid re-translating one-shot instructions
07afad5 target/arm: drop CF_LAST_IO/dc->condjump check
5dc7b9f hw/arm/armsse: Make SSE-300 use Cortex-M55
425d9fe hw/arm/armsse: Give SSE-300 its own Property array
ec5047b include/qemu/osdep.h: Move system includes to top
401fa67 osdep: protect qemu/osdep.h with extern "C"
7cf1c2e osdep: include glib-compat.h before other QEMU headers

=== OUTPUT BEGIN ===
1/7 Checking commit 7cf1c2efd765 (osdep: include glib-compat.h before other QEMU headers)
2/7 Checking commit 401fa67e0303 (osdep: protect qemu/osdep.h with extern "C")
WARNING: architecture specific defines should be avoided
#80: FILE: include/qemu/compiler.h:14:
+#ifdef __cplusplus

ERROR: storage class should be at the beginning of the declaration
#81: FILE: include/qemu/compiler.h:15:
+#define QEMU_EXTERN_C extern "C"

ERROR: storage class should be at the beginning of the declaration
#83: FILE: include/qemu/compiler.h:17:
+#define QEMU_EXTERN_C extern

WARNING: architecture specific defines should be avoided
#106: FILE: include/qemu/osdep.h:121:
+#ifdef __cplusplus

WARNING: architecture specific defines should be avoided
#117: FILE: include/qemu/osdep.h:735:
+#ifdef __cplusplus

total: 2 errors, 3 warnings, 56 lines checked

Patch 2/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/7 Checking commit ec5047b58ffc (include/qemu/osdep.h: Move system includes to top)
WARNING: architecture specific defines should be avoided
#37: FILE: include/qemu/osdep.h:111:
+#if defined(__linux__) && defined(__sparc__)

WARNING: architecture specific defines should be avoided
#49: FILE: include/qemu/osdep.h:123:
+#ifdef __APPLE__

total: 0 errors, 2 warnings, 50 lines checked

Patch 3/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/7 Checking commit 425d9fe70cce (hw/arm/armsse: Give SSE-300 its own Property array)
5/7 Checking commit 5dc7b9f9db8b (hw/arm/armsse: Make SSE-300 use Cortex-M55)
6/7 Checking commit 07afad503110 (target/arm: drop CF_LAST_IO/dc->condjump check)
7/7 Checking commit 72e612ff09f4 (accel/tcg: avoid re-translating one-shot instructions)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210417194205.17057-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PULL 0/7] queue of proposed rc4 fixes

[Philippe Mathieu-Daudé]

On 4/17/21 9:41 PM, Peter Maydell wrote:
> This pullreq contains fixes for the remaining "not fixed yet" issues
> in the 6.0 Planning page:
>  * Fix compile failures of C++ files with new glib headers
>  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
> 
> None of these are 100% rc4-worthy on their own, but taken all together
> I think they justify rolling another release candidate.

I wonder about this one for https://bugs.launchpad.net/qemu/+bug/1914236
"mptsas: remove unused MPTSASState.pending (CVE-2021-3392)"
https://www.mail-archive.com/qemu-devel@nongnu.org/msg799236.html
which is a respin of
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg02660.html
with Paolo's comment addressed.

> ----------------------------------------------------------------
> Fixes for rc4:
>  * Fix compile failures of C++ files with new glib headers
>  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
> 
> ----------------------------------------------------------------
> Alex Bennée (2):
>       target/arm: drop CF_LAST_IO/dc->condjump check
>       accel/tcg: avoid re-translating one-shot instructions
> 
> Paolo Bonzini (2):
>       osdep: include glib-compat.h before other QEMU headers
>       osdep: protect qemu/osdep.h with extern "C"
> 
> Peter Maydell (3):
>       include/qemu/osdep.h: Move system includes to top
>       hw/arm/armsse: Give SSE-300 its own Property array
>       hw/arm/armsse: Make SSE-300 use Cortex-M55
> 
>  include/qemu/compiler.h   |  6 ++++++
>  include/qemu/osdep.h      | 38 +++++++++++++++++++++++++++++---------
>  accel/tcg/translate-all.c |  2 +-
>  hw/arm/armsse.c           | 24 +++++++++++++++++++-----
>  target/arm/translate.c    |  5 -----
>  disas/arm-a64.cc          |  2 +-
>  disas/nanomips.cpp        |  2 +-
>  7 files changed, 57 insertions(+), 22 deletions(-)
> 

Re: [PULL 0/7] queue of proposed rc4 fixes

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> This pullreq contains fixes for the remaining "not fixed yet" issues
> in the 6.0 Planning page:
>  * Fix compile failures of C++ files with new glib headers
>  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
>
> None of these are 100% rc4-worthy on their own, but taken all together
> I think they justify rolling another release candidate.

If you are rolling it would be nice to include:

  checkpatch: Fix use of uninitialized value
  Message-Id: <161786467973.295167.5612704777283969903.stgit@bahia.lan>

just to avoid the messy warning in the CI checkpatch check.

>
> thanks
> -- PMM
>
> The following changes since commit 8fe9f1f891eff4e37f82622b7480ee748bf4af74:
>
>   Update version for v6.0.0-rc3 release (2021-04-14 22:06:18 +0100)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210417
>
> for you to fetch changes up to 277aed998ac2cd3649bf0e13b22f47769519eb61:
>
>   accel/tcg: avoid re-translating one-shot instructions (2021-04-17 18:51:14 +0100)
>
> ----------------------------------------------------------------
> Fixes for rc4:
>  * Fix compile failures of C++ files with new glib headers
>  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
>
> ----------------------------------------------------------------
> Alex Bennée (2):
>       target/arm: drop CF_LAST_IO/dc->condjump check
>       accel/tcg: avoid re-translating one-shot instructions
>
> Paolo Bonzini (2):
>       osdep: include glib-compat.h before other QEMU headers
>       osdep: protect qemu/osdep.h with extern "C"
>
> Peter Maydell (3):
>       include/qemu/osdep.h: Move system includes to top
>       hw/arm/armsse: Give SSE-300 its own Property array
>       hw/arm/armsse: Make SSE-300 use Cortex-M55
>
>  include/qemu/compiler.h   |  6 ++++++
>  include/qemu/osdep.h      | 38 +++++++++++++++++++++++++++++---------
>  accel/tcg/translate-all.c |  2 +-
>  hw/arm/armsse.c           | 24 +++++++++++++++++++-----
>  target/arm/translate.c    |  5 -----
>  disas/arm-a64.cc          |  2 +-
>  disas/nanomips.cpp        |  2 +-
>  7 files changed, 57 insertions(+), 22 deletions(-)


-- 
Alex Bennée

Re: [PULL 0/7] queue of proposed rc4 fixes

[Peter Maydell]

On Sun, 18 Apr 2021 at 17:31, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > This pullreq contains fixes for the remaining "not fixed yet" issues
> > in the 6.0 Planning page:
> >  * Fix compile failures of C++ files with new glib headers
> >  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
> >  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
> >
> > None of these are 100% rc4-worthy on their own, but taken all together
> > I think they justify rolling another release candidate.
>
> If you are rolling it would be nice to include:
>
>   checkpatch: Fix use of uninitialized value
>   Message-Id: <161786467973.295167.5612704777283969903.stgit@bahia.lan>
>
> just to avoid the messy warning in the CI checkpatch check.

I always ignore the CI checkpatch check in the github UI anyway :-)

-- PMM

Re: [PULL 0/7] queue of proposed rc4 fixes

[Philippe Mathieu-Daudé]

On 4/18/21 7:17 AM, Philippe Mathieu-Daudé wrote:
> On 4/17/21 9:41 PM, Peter Maydell wrote:
>> This pullreq contains fixes for the remaining "not fixed yet" issues
>> in the 6.0 Planning page:
>>  * Fix compile failures of C++ files with new glib headers
>>  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>>  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
>>
>> None of these are 100% rc4-worthy on their own, but taken all together
>> I think they justify rolling another release candidate.
> 
> I wonder about this one for https://bugs.launchpad.net/qemu/+bug/1914236
> "mptsas: remove unused MPTSASState.pending (CVE-2021-3392)"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg799236.html
> which is a respin of
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg02660.html
> with Paolo's comment addressed.

Actualized version:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg799620.html

This is not a new regression (present since QEMU v2.6.0) but is a
CVE...

Re: [PULL 0/7] queue of proposed rc4 fixes

[Thomas Huth]

On 17/04/2021 21.41, Peter Maydell wrote:
> This pullreq contains fixes for the remaining "not fixed yet" issues
> in the 6.0 Planning page:
>   * Fix compile failures of C++ files with new glib headers
>   * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>   * accel/tcg: Fix assertion failure executing from non-RAM with -icount
> 
> None of these are 100% rc4-worthy on their own, but taken all together
> I think they justify rolling another release candidate.

FWIW, I'm in favour of adding them, to make sure that QEMU v6.0 also 
compiles fine on future distros with the new glib headers - this will make 
debugging easier in the future (e.g. when bisecting or when asking people 
whether they can check whether a bug already occurred with v6.0 already 
etc.). Just my 0.02 € of course.

  Thomas

Re: [PULL 0/7] queue of proposed rc4 fixes

[Peter Maydell]

On Sat, 17 Apr 2021 at 20:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This pullreq contains fixes for the remaining "not fixed yet" issues
> in the 6.0 Planning page:
>  * Fix compile failures of C++ files with new glib headers
>  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
>
> None of these are 100% rc4-worthy on their own, but taken all together
> I think they justify rolling another release candidate.
>
> thanks
> -- PMM
>
> The following changes since commit 8fe9f1f891eff4e37f82622b7480ee748bf4af74:
>
>   Update version for v6.0.0-rc3 release (2021-04-14 22:06:18 +0100)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210417
>
> for you to fetch changes up to 277aed998ac2cd3649bf0e13b22f47769519eb61:
>
>   accel/tcg: avoid re-translating one-shot instructions (2021-04-17 18:51:14 +0100)
>
> ----------------------------------------------------------------
> Fixes for rc4:
>  * Fix compile failures of C++ files with new glib headers
>  * mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
>  * accel/tcg: Fix assertion failure executing from non-RAM with -icount
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM