qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/7] target-arm queue
@ 2015-11-10 13:51 Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 1/7] target-arm: Fix gdb singlestep handling in arm_debug_excp_handler() Peter Maydell
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

A small set of ARM patches, notably fixing bugs in breakpoint
and singlestep code, and repairing the long-broken highbank model.

The only other ARM thing I have on my radar for 2.5 is the Zynq
ADC controller, which I'll send separately if it makes it before
the freeze deadline.

thanks
-- PMM

The following changes since commit a8b4f9585a0bf5186fca793ce2c5d754cd8ec49a:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-11-10' into staging (2015-11-10 09:39:24 +0000)

are available in the git repository at:


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

for you to fetch changes up to 577bf808958d06497928c639efaa473bf8c5e099:

  target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code (2015-11-10 13:37:33 +0000)

----------------------------------------------------------------
target-arm queue:
 * fix bugs in gdb singlestep handling and breakpoints
 * minor code cleanup in arm_gic
 * clean up error messages in hw/arm/virt
 * fix highbank kernel booting by adding a board-setup blob

----------------------------------------------------------------
Andrew Jones (1):
      hw/arm/virt: error_report cleanups

Peter Crosthwaite (3):
      arm: boot: Add secure_board_setup flag
      arm: highbank: Defeature CPU override
      arm: highbank: Implement PSCI and dummy monitor

Sergey Fedorov (2):
      target-arm: Fix gdb singlestep handling in arm_debug_excp_handler()
      target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code

Wei Huang (1):
      hw/intc/arm_gic: Remove the definition of NUM_CPU

 hw/arm/boot.c          | 10 +++++-
 hw/arm/highbank.c      | 91 +++++++++++++++++++++++++++++++++++++-------------
 hw/arm/virt.c          | 10 +++---
 hw/intc/arm_gic.c      |  8 ++---
 include/hw/arm/arm.h   |  6 ++++
 target-arm/op_helper.c |  8 ++++-
 target-arm/translate.c | 25 ++++++++------
 7 files changed, 111 insertions(+), 47 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 1/7] target-arm: Fix gdb singlestep handling in arm_debug_excp_handler()
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
@ 2015-11-10 13:51 ` Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 2/7] hw/intc/arm_gic: Remove the definition of NUM_CPU Peter Maydell
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

From: Sergey Fedorov <serge.fdrv@gmail.com>

Do not raise a CPU exception if no CPU breakpoint has fired, since
singlestep is also done by generating a debug internal exception. This
fixes a bug with singlestepping in gdbstub.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Message-id: 1446726361-18328-1-git-send-email-serge.fdrv@gmail.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index b5db345..6cd54c8 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -917,7 +917,13 @@ void arm_debug_excp_handler(CPUState *cs)
         uint64_t pc = is_a64(env) ? env->pc : env->regs[15];
         bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
 
-        if (cpu_breakpoint_test(cs, pc, BP_GDB)) {
+        /* (1) GDB breakpoints should be handled first.
+         * (2) Do not raise a CPU exception if no CPU breakpoint has fired,
+         * since singlestep is also done by generating a debug internal
+         * exception.
+         */
+        if (cpu_breakpoint_test(cs, pc, BP_GDB)
+            || !cpu_breakpoint_test(cs, pc, BP_CPU)) {
             return;
         }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 2/7] hw/intc/arm_gic: Remove the definition of NUM_CPU
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 1/7] target-arm: Fix gdb singlestep handling in arm_debug_excp_handler() Peter Maydell
@ 2015-11-10 13:51 ` Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 3/7] arm: boot: Add secure_board_setup flag Peter Maydell
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

From: Wei Huang <wei@redhat.com>

arm_gic.c retrieves CPU number using either NUM_CPU(s) or s->num_cpu.
Such mixed-uses make source code inconsistent. This patch removes
NUM_CPU(s), which was defined for MPCore tweak long ago, and instead
favors s->num_cpu. The source is more consistent after this small tweak.

Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Wei Huang <wei@redhat.com>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Message-id: 1446744293-32365-1-git-send-email-wei@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gic.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 8bad132..d71aeb8 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -35,8 +35,6 @@ static const uint8_t gic_id[] = {
     0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1
 };
 
-#define NUM_CPU(s) ((s)->num_cpu)
-
 static inline int gic_get_current_cpu(GICState *s)
 {
     if (s->num_cpu > 1) {
@@ -64,7 +62,7 @@ void gic_update(GICState *s)
     int cpu;
     int cm;
 
-    for (cpu = 0; cpu < NUM_CPU(s); cpu++) {
+    for (cpu = 0; cpu < s->num_cpu; cpu++) {
         cm = 1 << cpu;
         s->current_pending[cpu] = 1023;
         if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1))
@@ -567,7 +565,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
         if (offset == 4)
             /* Interrupt Controller Type Register */
             return ((s->num_irq / 32) - 1)
-                    | ((NUM_CPU(s) - 1) << 5)
+                    | ((s->num_cpu - 1) << 5)
                     | (s->security_extn << 10);
         if (offset < 0x08)
             return 0;
@@ -1284,7 +1282,7 @@ static void arm_gic_realize(DeviceState *dev, Error **errp)
      * GIC v2 defines a larger memory region (0x1000) so this will need
      * to be extended when we implement A15.
      */
-    for (i = 0; i < NUM_CPU(s); i++) {
+    for (i = 0; i < s->num_cpu; i++) {
         s->backref[i] = s;
         memory_region_init_io(&s->cpuiomem[i+1], OBJECT(s), &gic_cpu_ops,
                               &s->backref[i], "gic_cpu", 0x100);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 3/7] arm: boot: Add secure_board_setup flag
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 1/7] target-arm: Fix gdb singlestep handling in arm_debug_excp_handler() Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 2/7] hw/intc/arm_gic: Remove the definition of NUM_CPU Peter Maydell
@ 2015-11-10 13:51 ` Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 4/7] arm: highbank: Defeature CPU override Peter Maydell
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

From: Peter Crosthwaite <crosthwaitepeter@gmail.com>

Add a flag that when set, will cause the primary CPU to start in secure
mode, even if the overall boot is non-secure. This is useful for when
there is a board-setup blob that needs to run from secure mode, but
device and secondary CPU init should still be done as-normal for a non-
secure boot.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Message-id: d1170774d5446d715fced7739edfc61a5be931f9.1447007690.git.crosthwaite.peter@gmail.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c        | 10 +++++++++-
 include/hw/arm/arm.h |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index b0879a5..75f69bf 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -11,6 +11,7 @@
 #include "hw/hw.h"
 #include "hw/arm/arm.h"
 #include "hw/arm/linux-boot-if.h"
+#include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -495,7 +496,8 @@ static void do_cpu_reset(void *opaque)
                 }
 
                 /* Set to non-secure if not a secure boot */
-                if (!info->secure_boot) {
+                if (!info->secure_boot &&
+                    (cs != first_cpu || !info->secure_board_setup)) {
                     /* Linux expects non-secure state */
                     env->cp15.scr_el3 |= SCR_NS;
                 }
@@ -598,6 +600,12 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
     struct arm_boot_info *info =
         container_of(n, struct arm_boot_info, load_kernel_notifier);
 
+    /* The board code is not supposed to set secure_board_setup unless
+     * running its code in secure mode is actually possible, and KVM
+     * doesn't support secure.
+     */
+    assert(!(info->secure_board_setup && kvm_enabled()));
+
     /* Load the kernel.  */
     if (!info->kernel_filename || info->firmware_loaded) {
 
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index 67ba7db..c26b0e3 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -97,6 +97,12 @@ struct arm_boot_info {
     hwaddr board_setup_addr;
     void (*write_board_setup)(ARMCPU *cpu,
                               const struct arm_boot_info *info);
+
+    /* If set, the board specific loader/setup blob will be run from secure
+     * mode, regardless of secure_boot. The blob becomes responsible for
+     * changing to non-secure state if implementing a non-secure boot
+     */
+    bool secure_board_setup;
 };
 
 /**
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 4/7] arm: highbank: Defeature CPU override
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
                   ` (2 preceding siblings ...)
  2015-11-10 13:51 ` [Qemu-devel] [PULL 3/7] arm: boot: Add secure_board_setup flag Peter Maydell
@ 2015-11-10 13:51 ` Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 5/7] arm: highbank: Implement PSCI and dummy monitor Peter Maydell
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

From: Peter Crosthwaite <crosthwaitepeter@gmail.com>

This board should not support CPU model override. This allows for
easier patching of the board with being able to rely on the CPU
type being correct.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Message-id: 471a61e049c7ca6e82f5ef6668889a1d518c7e00.1447007690.git.crosthwaite.peter@gmail.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/highbank.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index be04b27..f2e248b 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -223,15 +223,13 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
     MemoryRegion *sysmem;
     char *sysboot_filename;
 
-    if (!cpu_model) {
-        switch (machine_id) {
-        case CALXEDA_HIGHBANK:
-            cpu_model = "cortex-a9";
-            break;
-        case CALXEDA_MIDWAY:
-            cpu_model = "cortex-a15";
-            break;
-        }
+    switch (machine_id) {
+    case CALXEDA_HIGHBANK:
+        cpu_model = "cortex-a9";
+        break;
+    case CALXEDA_MIDWAY:
+        cpu_model = "cortex-a15";
+        break;
     }
 
     for (n = 0; n < smp_cpus; n++) {
@@ -240,11 +238,6 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         ARMCPU *cpu;
         Error *err = NULL;
 
-        if (!oc) {
-            error_report("Unable to find CPU definition");
-            exit(1);
-        }
-
         cpuobj = object_new(object_class_get_name(oc));
         cpu = ARM_CPU(cpuobj);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 5/7] arm: highbank: Implement PSCI and dummy monitor
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
                   ` (3 preceding siblings ...)
  2015-11-10 13:51 ` [Qemu-devel] [PULL 4/7] arm: highbank: Defeature CPU override Peter Maydell
@ 2015-11-10 13:51 ` Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 6/7] hw/arm/virt: error_report cleanups Peter Maydell
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

From: Peter Crosthwaite <crosthwaitepeter@gmail.com>

Firstly, enable monitor mode and PSCI, both of which are features of
this board.

In addition to PSCI, this board also uses SMC for cache maintenance
ops. This means we need a secure monitor to catch these and nop them.
Use the ARM boot board-setup feature to implement this. The SMC trap
implements the needed nop while all other traps will pen the CPU.

As a KVM CPU cannot run in secure mode, do not do the board-setup if
not running TCG. Report a warning explaining the limitation in this
case.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Message-id: 0fd0d12f0fa666c86616c89447861a70dbe27312.1447007690.git.crosthwaite.peter@gmail.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/highbank.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 10 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index f2e248b..85ae69e 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -22,6 +22,7 @@
 #include "hw/devices.h"
 #include "hw/loader.h"
 #include "net/net.h"
+#include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "sysemu/block-backend.h"
@@ -32,10 +33,52 @@
 #define SMP_BOOT_REG            0x40
 #define MPCORE_PERIPHBASE       0xfff10000
 
+#define MVBAR_ADDR              0x200
+
 #define NIRQ_GIC                160
 
 /* Board init.  */
 
+/* MVBAR_ADDR is limited by precision of movw */
+
+QEMU_BUILD_BUG_ON(MVBAR_ADDR >= (1 << 16));
+
+#define ARMV7_IMM16(x) (extract32((x),  0, 12) | \
+                        extract32((x), 12,  4) << 16)
+
+static void hb_write_board_setup(ARMCPU *cpu,
+                                 const struct arm_boot_info *info)
+{
+    int n;
+    uint32_t board_setup_blob[] = {
+        /* MVBAR_ADDR */
+        /* Default unimplemented and unused vectors to spin. Makes it
+         * easier to debug (as opposed to the CPU running away).
+         */
+        0xeafffffe, /* notused1: b notused */
+        0xeafffffe, /* notused2: b notused */
+        0xe1b0f00e, /* smc: movs pc, lr - exception return */
+        0xeafffffe, /* prefetch_abort: b prefetch_abort */
+        0xeafffffe, /* data_abort: b data_abort */
+        0xeafffffe, /* notused3: b notused3 */
+        0xeafffffe, /* irq: b irq */
+        0xeafffffe, /* fiq: b fiq */
+#define BOARD_SETUP_ADDR (MVBAR_ADDR + 8 * sizeof(uint32_t))
+        0xe3000000 + ARMV7_IMM16(MVBAR_ADDR), /* movw r0, MVBAR_ADDR */
+        0xee0c0f30, /* mcr p15, 0, r0, c12, c0, 1 - set MVBAR */
+        0xee110f11, /* mrc p15, 0, r0, c1 , c1, 0 - get SCR */
+        0xe3810001, /* orr r0, #1 - set NS */
+        0xee010f11, /* mcr p15, 0, r0, c1 , c1, 0 - set SCR */
+        0xe1600070, /* smc - go to monitor mode to flush NS change */
+        0xe12fff1e, /* bx lr - return to caller */
+    };
+    for (n = 0; n < ARRAY_SIZE(board_setup_blob); n++) {
+        board_setup_blob[n] = tswap32(board_setup_blob[n]);
+    }
+    rom_add_blob_fixed("board-setup", board_setup_blob,
+                       sizeof(board_setup_blob), MVBAR_ADDR);
+}
+
 static void hb_write_secondary(ARMCPU *cpu, const struct arm_boot_info *info)
 {
     int n;
@@ -241,16 +284,13 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         cpuobj = object_new(object_class_get_name(oc));
         cpu = ARM_CPU(cpuobj);
 
-        /* By default A9 and A15 CPUs have EL3 enabled.  This board does not
-         * currently support EL3 so the CPU EL3 property is disabled before
-         * realization.
-         */
-        if (object_property_find(cpuobj, "has_el3", NULL)) {
-            object_property_set_bool(cpuobj, false, "has_el3", &err);
-            if (err) {
-                error_report_err(err);
-                exit(1);
-            }
+        object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_SMC,
+                                "psci-conduit", &error_abort);
+
+        if (n) {
+            /* Secondary CPUs start in PSCI powered-down state */
+            object_property_set_bool(cpuobj, true,
+                                     "start-powered-off", &error_abort);
         }
 
         if (object_property_find(cpuobj, "reset-cbar", NULL)) {
@@ -371,6 +411,16 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
     highbank_binfo.loader_start = 0;
     highbank_binfo.write_secondary_boot = hb_write_secondary;
     highbank_binfo.secondary_cpu_reset_hook = hb_reset_secondary;
+    if (!kvm_enabled()) {
+        highbank_binfo.board_setup_addr = BOARD_SETUP_ADDR;
+        highbank_binfo.write_board_setup = hb_write_board_setup;
+        highbank_binfo.secure_board_setup = true;
+    } else {
+        error_report("WARNING: cannot load built-in Monitor support "
+                     "if KVM is enabled. Some guests (such as Linux) "
+                     "may not boot.");
+    }
+
     arm_load_kernel(ARM_CPU(first_cpu), &highbank_binfo);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 6/7] hw/arm/virt: error_report cleanups
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
                   ` (4 preceding siblings ...)
  2015-11-10 13:51 ` [Qemu-devel] [PULL 5/7] arm: highbank: Implement PSCI and dummy monitor Peter Maydell
@ 2015-11-10 13:51 ` Peter Maydell
  2015-11-10 13:51 ` [Qemu-devel] [PULL 7/7] target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code Peter Maydell
  2015-11-10 16:38 ` [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
  7 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

From: Andrew Jones <drjones@redhat.com>

Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1446909925-12201-1-git-send-email-drjones@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 77d9267..9c6792c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -941,8 +941,8 @@ static void machvirt_init(MachineState *machine)
     if (!gic_version) {
         gic_version = kvm_arm_vgic_probe();
         if (!gic_version) {
-            error_report("Unable to determine GIC version supported by host\n"
-                         "Probably KVM acceleration is not supported\n");
+            error_report("Unable to determine GIC version supported by host");
+            error_printf("KVM acceleration is probably not supported\n");
             exit(1);
         }
     }
@@ -990,7 +990,7 @@ static void machvirt_init(MachineState *machine)
         char *cpuopts = g_strdup(cpustr[1]);
 
         if (!oc) {
-            fprintf(stderr, "Unable to find CPU definition\n");
+            error_report("Unable to find CPU definition");
             exit(1);
         }
         cpuobj = object_new(object_class_get_name(oc));
@@ -1126,8 +1126,8 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
     } else if (!strcmp(value, "host")) {
         vms->gic_version = 0; /* Will probe later */
     } else {
-        error_report("Invalid gic-version option value\n"
-                     "Allowed values are: 3, 2, host\n");
+        error_report("Invalid gic-version option value");
+        error_printf("Allowed gic-version values are: 3, 2, host\n");
         exit(1);
     }
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 7/7] target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
                   ` (5 preceding siblings ...)
  2015-11-10 13:51 ` [Qemu-devel] [PULL 6/7] hw/arm/virt: error_report cleanups Peter Maydell
@ 2015-11-10 13:51 ` Peter Maydell
  2015-11-10 16:38 ` [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
  7 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 13:51 UTC (permalink / raw)
  To: qemu-devel

From: Sergey Fedorov <serge.fdrv@gmail.com>

AArch32 translation code does not distinguish between DISAS_UPDATE and
DISAS_JUMP. Thus, we cannot use any of them without first updating PC in
CPU state. Furthermore, it is too complicated to update PC in CPU state
before PC gets updated in disas context. So it is hardly possible to
correctly end TB early if is is not likely to be executed before calling
disas_*_insn(), e.g. just after calling breakpoint check helper.

Modify DISAS_UPDATE and DISAS_JUMP usage in AArch32 translation and
apply to them the same semantic as AArch64 translation does:
 - DISAS_UPDATE: update PC in CPU state when finishing translation
 - DISAS_JUMP:   preserve current PC value in CPU state when finishing
                 translation

This patch fixes a bug in AArch32 breakpoint handling: when
check_breakpoints helper does not generate an exception, ending the TB
early with DISAS_UPDATE couldn't update PC in CPU state and execution
hangs.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Message-id: 1447097859-586-1-git-send-email-serge.fdrv@gmail.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index ff262a2..a56f7fe 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -870,7 +870,7 @@ static inline void gen_bx_im(DisasContext *s, uint32_t addr)
 {
     TCGv_i32 tmp;
 
-    s->is_jmp = DISAS_UPDATE;
+    s->is_jmp = DISAS_JUMP;
     if (s->thumb != (addr & 1)) {
         tmp = tcg_temp_new_i32();
         tcg_gen_movi_i32(tmp, addr & 1);
@@ -883,7 +883,7 @@ static inline void gen_bx_im(DisasContext *s, uint32_t addr)
 /* Set PC and Thumb state from var.  var is marked as dead.  */
 static inline void gen_bx(DisasContext *s, TCGv_i32 var)
 {
-    s->is_jmp = DISAS_UPDATE;
+    s->is_jmp = DISAS_JUMP;
     tcg_gen_andi_i32(cpu_R[15], var, ~1);
     tcg_gen_andi_i32(var, var, 1);
     store_cpu_field(var, thumb);
@@ -1062,7 +1062,7 @@ static void gen_exception_insn(DisasContext *s, int offset, int excp,
 static inline void gen_lookup_tb(DisasContext *s)
 {
     tcg_gen_movi_i32(cpu_R[15], s->pc & ~1);
-    s->is_jmp = DISAS_UPDATE;
+    s->is_jmp = DISAS_JUMP;
 }
 
 static inline void gen_add_data_offset(DisasContext *s, unsigned int insn,
@@ -4096,7 +4096,7 @@ static void gen_exception_return(DisasContext *s, TCGv_i32 pc)
     tmp = load_cpu_field(spsr);
     gen_set_cpsr(tmp, CPSR_ERET_MASK);
     tcg_temp_free_i32(tmp);
-    s->is_jmp = DISAS_UPDATE;
+    s->is_jmp = DISAS_JUMP;
 }
 
 /* Generate a v6 exception return.  Marks both values as dead.  */
@@ -4105,7 +4105,7 @@ static void gen_rfe(DisasContext *s, TCGv_i32 pc, TCGv_i32 cpsr)
     gen_set_cpsr(cpsr, CPSR_ERET_MASK);
     tcg_temp_free_i32(cpsr);
     store_reg(s, 15, pc);
-    s->is_jmp = DISAS_UPDATE;
+    s->is_jmp = DISAS_JUMP;
 }
 
 static void gen_nop_hint(DisasContext *s, int val)
@@ -9035,7 +9035,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
                     tmp = load_cpu_field(spsr);
                     gen_set_cpsr(tmp, CPSR_ERET_MASK);
                     tcg_temp_free_i32(tmp);
-                    s->is_jmp = DISAS_UPDATE;
+                    s->is_jmp = DISAS_JUMP;
                 }
             }
             break;
@@ -11355,7 +11355,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
             /* We always get here via a jump, so know we are not in a
                conditional execution block.  */
             gen_exception_internal(EXCP_KERNEL_TRAP);
-            dc->is_jmp = DISAS_UPDATE;
+            dc->is_jmp = DISAS_EXC;
             break;
         }
 #else
@@ -11363,7 +11363,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
             /* We always get here via a jump, so know we are not in a
                conditional execution block.  */
             gen_exception_internal(EXCP_EXCEPTION_EXIT);
-            dc->is_jmp = DISAS_UPDATE;
+            dc->is_jmp = DISAS_EXC;
             break;
         }
 #endif
@@ -11497,7 +11497,8 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
             }
             gen_set_label(dc->condlabel);
         }
-        if (dc->condjmp || !dc->is_jmp) {
+        if (dc->condjmp || dc->is_jmp == DISAS_NEXT ||
+            dc->is_jmp == DISAS_UPDATE) {
             gen_set_pc_im(dc, dc->pc);
             dc->condjmp = 0;
         }
@@ -11533,9 +11534,11 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
         case DISAS_NEXT:
             gen_goto_tb(dc, 1, dc->pc);
             break;
-        default:
-        case DISAS_JUMP:
         case DISAS_UPDATE:
+            gen_set_pc_im(dc, dc->pc);
+            /* fall through */
+        case DISAS_JUMP:
+        default:
             /* indicate that the hash table must be used to find the next TB */
             tcg_gen_exit_tb(0);
             break;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/7] target-arm queue
  2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
                   ` (6 preceding siblings ...)
  2015-11-10 13:51 ` [Qemu-devel] [PULL 7/7] target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code Peter Maydell
@ 2015-11-10 16:38 ` Peter Maydell
  2015-11-10 17:12   ` Peter Crosthwaite
  7 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 16:38 UTC (permalink / raw)
  To: QEMU Developers

On 10 November 2015 at 13:51, Peter Maydell <peter.maydell@linaro.org> wrote:
> A small set of ARM patches, notably fixing bugs in breakpoint
> and singlestep code, and repairing the long-broken highbank model.
>
> The only other ARM thing I have on my radar for 2.5 is the Zynq
> ADC controller, which I'll send separately if it makes it before
> the freeze deadline.
>
> thanks
> -- PMM
>
> The following changes since commit a8b4f9585a0bf5186fca793ce2c5d754cd8ec49a:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-11-10' into staging (2015-11-10 09:39:24 +0000)
>
> are available in the git repository at:
>
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20151110
>
> for you to fetch changes up to 577bf808958d06497928c639efaa473bf8c5e099:
>
>   target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code (2015-11-10 13:37:33 +0000)
>
> ----------------------------------------------------------------
> target-arm queue:
>  * fix bugs in gdb singlestep handling and breakpoints
>  * minor code cleanup in arm_gic
>  * clean up error messages in hw/arm/virt
>  * fix highbank kernel booting by adding a board-setup blob
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/7] target-arm queue
  2015-11-10 16:38 ` [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
@ 2015-11-10 17:12   ` Peter Crosthwaite
  2015-11-10 17:13     ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Crosthwaite @ 2015-11-10 17:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On Tue, Nov 10, 2015 at 8:38 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 10 November 2015 at 13:51, Peter Maydell <peter.maydell@linaro.org> wrote:
>> A small set of ARM patches, notably fixing bugs in breakpoint
>> and singlestep code, and repairing the long-broken highbank model.
>>
>> The only other ARM thing I have on my radar for 2.5 is the Zynq
>> ADC controller, which I'll send separately if it makes it before
>> the freeze deadline.
>>

It is on list I think. I don't see further review:

[PATCH for-2.5 v4 1/1] hw/misc: Add support for ADC controller in
Xilinx Zynq 7000

Regards,
Peter

>> thanks
>> -- PMM
>>
>> The following changes since commit a8b4f9585a0bf5186fca793ce2c5d754cd8ec49a:
>>
>>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-11-10' into staging (2015-11-10 09:39:24 +0000)
>>
>> are available in the git repository at:
>>
>>
>>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20151110
>>
>> for you to fetch changes up to 577bf808958d06497928c639efaa473bf8c5e099:
>>
>>   target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code (2015-11-10 13:37:33 +0000)
>>
>> ----------------------------------------------------------------
>> target-arm queue:
>>  * fix bugs in gdb singlestep handling and breakpoints
>>  * minor code cleanup in arm_gic
>>  * clean up error messages in hw/arm/virt
>>  * fix highbank kernel booting by adding a board-setup blob
>>
>
> Applied, thanks.
>
> -- PMM
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/7] target-arm queue
  2015-11-10 17:12   ` Peter Crosthwaite
@ 2015-11-10 17:13     ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2015-11-10 17:13 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: QEMU Developers

On 10 November 2015 at 17:12, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Tue, Nov 10, 2015 at 8:38 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 10 November 2015 at 13:51, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> A small set of ARM patches, notably fixing bugs in breakpoint
>>> and singlestep code, and repairing the long-broken highbank model.
>>>
>>> The only other ARM thing I have on my radar for 2.5 is the Zynq
>>> ADC controller, which I'll send separately if it makes it before
>>> the freeze deadline.
>>>
>
> It is on list I think. I don't see further review:
>
> [PATCH for-2.5 v4 1/1] hw/misc: Add support for ADC controller in
> Xilinx Zynq 7000

Ah yes, found it -- not sure why my search didn't turn it up earlier.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-11-10 17:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-10 13:51 [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
2015-11-10 13:51 ` [Qemu-devel] [PULL 1/7] target-arm: Fix gdb singlestep handling in arm_debug_excp_handler() Peter Maydell
2015-11-10 13:51 ` [Qemu-devel] [PULL 2/7] hw/intc/arm_gic: Remove the definition of NUM_CPU Peter Maydell
2015-11-10 13:51 ` [Qemu-devel] [PULL 3/7] arm: boot: Add secure_board_setup flag Peter Maydell
2015-11-10 13:51 ` [Qemu-devel] [PULL 4/7] arm: highbank: Defeature CPU override Peter Maydell
2015-11-10 13:51 ` [Qemu-devel] [PULL 5/7] arm: highbank: Implement PSCI and dummy monitor Peter Maydell
2015-11-10 13:51 ` [Qemu-devel] [PULL 6/7] hw/arm/virt: error_report cleanups Peter Maydell
2015-11-10 13:51 ` [Qemu-devel] [PULL 7/7] target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code Peter Maydell
2015-11-10 16:38 ` [Qemu-devel] [PULL 0/7] target-arm queue Peter Maydell
2015-11-10 17:12   ` Peter Crosthwaite
2015-11-10 17:13     ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).