From 5c629f4ff4dc9ae79cc732f59a8df15ede796ff7 Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Tue, 10 Nov 2015 13:37:32 +0000 Subject: target-arm: Fix gdb singlestep handling in arm_debug_excp_handler() 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 Message-id: 1446726361-18328-1-git-send-email-serge.fdrv@gmail.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- 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 b5db345ccd..6cd54c8f7a 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; } -- cgit v1.2.3 From b95690c9beaffd95edf91eb67829dc1e0a81e666 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Tue, 10 Nov 2015 13:37:33 +0000 Subject: hw/intc/arm_gic: Remove the definition of NUM_CPU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Wei Huang Reviewed-by: Michael Tokarev Message-id: 1446744293-32365-1-git-send-email-wei@redhat.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- 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 8bad132d5a..d71aeb8a2a 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); -- cgit v1.2.3 From baf6b6815ba9ae8255eefd1ddf15216d53da34b5 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Tue, 10 Nov 2015 13:37:33 +0000 Subject: arm: boot: Add secure_board_setup flag 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 Message-id: d1170774d5446d715fced7739edfc61a5be931f9.1447007690.git.crosthwaite.peter@gmail.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- 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 b0879a5e76..75f69bfe01 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 67ba7db3bb..c26b0e357f 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; }; /** -- cgit v1.2.3 From dca6eeed8c2a1c131d161139428dd18a35e58b03 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Tue, 10 Nov 2015 13:37:33 +0000 Subject: arm: highbank: Defeature CPU override 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 Signed-off-by: Peter Crosthwaite Message-id: 471a61e049c7ca6e82f5ef6668889a1d518c7e00.1447007690.git.crosthwaite.peter@gmail.com Signed-off-by: Peter Maydell --- 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 be04b27230..f2e248b044 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); -- cgit v1.2.3 From 40340e5f221935723bffbca305f3090e8866c818 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Tue, 10 Nov 2015 13:37:33 +0000 Subject: arm: highbank: Implement PSCI and dummy monitor 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 Signed-off-by: Peter Crosthwaite Message-id: 0fd0d12f0fa666c86616c89447861a70dbe27312.1447007690.git.crosthwaite.peter@gmail.com Signed-off-by: Peter Maydell --- 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 f2e248b044..85ae69efd9 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); } -- cgit v1.2.3 From faa811f6de44d58180f5d235787678dcdd4b2e9d Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Tue, 10 Nov 2015 13:37:33 +0000 Subject: hw/arm/virt: error_report cleanups Signed-off-by: Andrew Jones Reviewed-by: Markus Armbruster Message-id: 1446909925-12201-1-git-send-email-drjones@redhat.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- 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 77d9267599..9c6792cea1 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); } } -- cgit v1.2.3 From 577bf808958d06497928c639efaa473bf8c5e099 Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Tue, 10 Nov 2015 13:37:33 +0000 Subject: target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code 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 Message-id: 1447097859-586-1-git-send-email-serge.fdrv@gmail.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- 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 ff262a2572..a56f7fe532 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; -- cgit v1.2.3