From a75a52d62418dafe462be4fe30485501d1010bb9 Mon Sep 17 00:00:00 2001
From: Victor Kamensky <kamensky@cisco.com>
Date: Fri, 23 Mar 2018 18:26:45 +0000
Subject: [PATCH 01/10] arm/translate-a64: treat DISAS_UPDATE as variant of
DISAS_EXIT
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
In OE project 4.15 linux kernel boot hang was observed under
single cpu aarch64 qemu. Kernel code was in a loop waiting for
vtimer arrival, spinning in TC generated blocks, while interrupt
was pending unprocessed. This happened because when qemu tried to
handle vtimer interrupt target had interrupts disabled, as
result flag indicating TCG exit, cpu->icount_decr.u16.high,
was cleared but arm_cpu_exec_interrupt function did not call
arm_cpu_do_interrupt to process interrupt. Later when target
reenabled interrupts, it happened without exit into main loop, so
following code that waited for result of interrupt execution
run in infinite loop.
To solve the problem instructions that operate on CPU sys state
(i.e enable/disable interrupt), and marked as DISAS_UPDATE,
should be considered as DISAS_EXIT variant, and should be
forced to exit back to main loop so qemu will have a chance
processing pending CPU state updates, including pending
interrupts.
This change brings consistency with how DISAS_UPDATE is treated
in aarch32 case.
CC: Peter Maydell <peter.maydell@linaro.org>
CC: Alex Bennée <alex.bennee@linaro.org>
CC: qemu-stable@nongnu.org
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Victor Kamensky <kamensky@cisco.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 1521526368-1996-1-git-send-email-kamensky@cisco.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-a64.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 31ff0479e6..327513ef40 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -13378,12 +13378,12 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
case DISAS_UPDATE:
gen_a64_set_pc_im(dc->pc);
/* fall through */
- case DISAS_JUMP:
- tcg_gen_lookup_and_goto_ptr();
- break;
case DISAS_EXIT:
tcg_gen_exit_tb(0);
break;
+ case DISAS_JUMP:
+ tcg_gen_lookup_and_goto_ptr();
+ break;
case DISAS_NORETURN:
case DISAS_SWI:
break;
From 544156efcf4d807507d223075c26702a1254880e Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Fri, 23 Mar 2018 18:26:45 +0000
Subject: [PATCH 02/10] sdhci: fix incorrect use of Error *
Detected by Coverity (CID 1386072, 1386073, 1386076, 1386077). local_err
was unused, and this made the static analyzer unhappy.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20180320151355.25854-1-pbonzini@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/sd/sdhci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1b828b104d..63c44a4ee8 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1474,7 +1474,7 @@ static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
Error *local_err = NULL;
sdhci_initfn(s);
- sdhci_common_realize(s, errp);
+ sdhci_common_realize(s, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -1556,7 +1556,7 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
Error *local_err = NULL;
- sdhci_common_realize(s, errp);
+ sdhci_common_realize(s, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
From a2e2d7fc46fd8be875035d9bb5c64788389f65c2 Mon Sep 17 00:00:00 2001
From: Peter Maydell <peter.maydell@linaro.org>
Date: Fri, 23 Mar 2018 18:26:45 +0000
Subject: [PATCH 03/10] hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and
ICC_RPR accesses
If the GIC has the security extension support enabled, then a
non-secure access to ICC_PMR must take account of the non-secure
view of interrupt priorities, where real priorities 0x00..0x7f
are secure-only and not visible to the non-secure guest, and
priorities 0x80..0xff are shown to the guest as if they were
0x00..0xff. We had the logic here wrong:
* on reads, the priority is in the secure range if bit 7
is clear, not if it is set
* on writes, we want to set bit 7, not mask everything else
Our ICC_RPR read code had the same error as ICC_PMR.
(Compare the GICv3 spec pseudocode functions ICC_RPR_EL1
and ICC_PMR_EL1.)
Fixes: https://bugs.launchpad.net/qemu/+bug/1748434
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20180315133441.24149-1-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_cpuif.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 5cbafaf497..26f5eeda94 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -836,7 +836,7 @@ static uint64_t icc_pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
/* NS access and Group 0 is inaccessible to NS: return the
* NS view of the current priority
*/
- if (value & 0x80) {
+ if ((value & 0x80) == 0) {
/* Secure priorities not visible to NS */
value = 0;
} else if (value != 0xff) {
@@ -871,7 +871,7 @@ static void icc_pmr_write(CPUARMState *env, const ARMCPRegInfo *ri,
/* Current PMR in the secure range, don't allow NS to change it */
return;
}
- value = (value >> 1) & 0x80;
+ value = (value >> 1) | 0x80;
}
cs->icc_pmr_el1 = value;
gicv3_cpuif_update(cs);
@@ -1609,7 +1609,7 @@ static uint64_t icc_rpr_read(CPUARMState *env, const ARMCPRegInfo *ri)
if (arm_feature(env, ARM_FEATURE_EL3) &&
!arm_is_secure(env) && (env->cp15.scr_el3 & SCR_FIQ)) {
/* NS GIC access and Group 0 is inaccessible to NS */
- if (prio & 0x80) {
+ if ((prio & 0x80) == 0) {
/* NS mustn't see priorities in the Secure half of the range */
prio = 0;
} else if (prio != 0xff) {
From 2b0b93210a06d8a6d99f8015b79ee3aabed8f29a Mon Sep 17 00:00:00 2001
From: Peter Maydell <peter.maydell@linaro.org>
Date: Fri, 23 Mar 2018 18:26:45 +0000
Subject: [PATCH 04/10] hw/arm/bcm2836: Use the Cortex-A7 instead of Cortex-A15
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The BCM2836 uses a Cortex-A7, not a Cortex-A15. Update the device to
use the correct CPU.
https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2836/QA7_rev3.4.pdf
When the BCM2836 was introduced (bad5623690b) the Cortex-A7 was not
available, so the very similar Cortex-A15 was used. Since dcf578ed8ce
we can model the correct core.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180319110215.16755-1-peter.maydell@linaro.org
---
hw/arm/bcm2836.c | 2 +-
hw/arm/raspi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 3e7e8ca791..3c4b44a53e 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -32,7 +32,7 @@ struct BCM283XInfo {
static const BCM283XInfo bcm283x_socs[] = {
{
.name = TYPE_BCM2836,
- .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
+ .cpu_type = ARM_CPU_TYPE_NAME("cortex-a7"),
.clusterid = 0xf,
},
#ifdef TARGET_AARCH64
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 06f1e08ca9..955a7c4e80 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -226,7 +226,7 @@ static void raspi2_machine_init(MachineClass *mc)
mc->no_parallel = 1;
mc->no_floppy = 1;
mc->no_cdrom = 1;
- mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
+ mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
mc->max_cpus = BCM283X_NCPUS;
mc->min_cpus = BCM283X_NCPUS;
mc->default_cpus = BCM283X_NCPUS;
From 478a573a7d079a463273db7f9e7b0d460120c2dd Mon Sep 17 00:00:00 2001
From: Trent Piepho <tpiepho@impinj.com>
Date: Fri, 23 Mar 2018 18:26:45 +0000
Subject: [PATCH 05/10] i.MX: Support serial RS-232 break properly
Linux does not detect a break from this IMX serial driver as a magic
sysrq. Nor does it note a break in the port error counts.
The former is because the Linux driver uses the BRCD bit in the USR2
register to trigger the RS-232 break handler in the kernel, which is
where sysrq hooks in. The emulated UART was not setting this status
bit.
The latter is because the Linux driver expects, in addition to the BRK
bit, that the ERR bit is set when a break is read in the FIFO. A break
should also count as a frame error, so add that bit too.
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
Message-id: 20180320013657.25038-1-tpiepho@impinj.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/char/imx_serial.c | 5 ++++-
include/hw/char/imx_serial.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index 1e5540472b..0747db9f2b 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -308,6 +308,9 @@ static void imx_put_data(void *opaque, uint32_t value)
s->usr2 |= USR2_RDR;
s->uts1 &= ~UTS1_RXEMPTY;
s->readbuff = value;
+ if (value & URXD_BRK) {
+ s->usr2 |= USR2_BRCD;
+ }
imx_update(s);
}
@@ -319,7 +322,7 @@ static void imx_receive(void *opaque, const uint8_t *buf, int size)
static void imx_event(void *opaque, int event)
{
if (event == CHR_EVENT_BREAK) {
- imx_put_data(opaque, URXD_BRK);
+ imx_put_data(opaque, URXD_BRK | URXD_FRMERR | URXD_ERR);
}
}
diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index 5b99cee7cf..ee80da12e6 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -26,6 +26,7 @@
#define URXD_CHARRDY (1<<15) /* character read is valid */
#define URXD_ERR (1<<14) /* Character has error */
+#define URXD_FRMERR (1<<12) /* Character has frame error */
#define URXD_BRK (1<<11) /* Break received */
#define USR1_PARTYER (1<<15) /* Parity Error */
From dfadc3bfb458efefb72e13a57b62f138c464a577 Mon Sep 17 00:00:00 2001
From: Wei Huang <wei@redhat.com>
Date: Fri, 23 Mar 2018 18:26:46 +0000
Subject: [PATCH 06/10] mach-virt: Set VM's SMBIOS system version to mc->name
Instead of using "1.0" as the system version of SMBIOS, we should use
mc->name for mach-virt machine type to be consistent other architectures.
With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will
show:
Handle 0x0100, DMI type 1, 27 bytes
System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: virt-2.12
Serial Number: Not Specified
...
instead of:
Handle 0x0100, DMI type 1, 27 bytes
System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: 1.0
Serial Number: Not Specified
...
For backward compatibility, we allow older machine types to keep "1.0"
as the default system version.
Signed-off-by: Wei Huang <wei@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20180322212318.7182-1-wei@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 8 +++++++-
include/hw/arm/virt.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2c07245047..94dcb125d3 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1132,6 +1132,8 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
static void virt_build_smbios(VirtMachineState *vms)
{
+ MachineClass *mc = MACHINE_GET_CLASS(vms);
+ VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
uint8_t *smbios_tables, *smbios_anchor;
size_t smbios_tables_len, smbios_anchor_len;
const char *product = "QEMU Virtual Machine";
@@ -1145,7 +1147,8 @@ static void virt_build_smbios(VirtMachineState *vms)
}
smbios_set_defaults("QEMU", product,
- "1.0", false, true, SMBIOS_ENTRY_POINT_30);
+ vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
+ true, SMBIOS_ENTRY_POINT_30);
smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
&smbios_anchor, &smbios_anchor_len);
@@ -1646,8 +1649,11 @@ static void virt_2_11_instance_init(Object *obj)
static void virt_machine_2_11_options(MachineClass *mc)
{
+ VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
virt_machine_2_12_options(mc);
SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_11);
+ vmc->smbios_old_sys_ver = true;
}
DEFINE_VIRT_MACHINE(2, 11)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 33b0ff3892..ba0c1a4faa 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -85,6 +85,7 @@ typedef struct {
bool no_its;
bool no_pmu;
bool claim_edge_triggered_timers;
+ bool smbios_old_sys_ver;
} VirtMachineClass;
typedef struct {
From c900a2e62dd6dde11c8f5249b638caad05bb15be Mon Sep 17 00:00:00 2001
From: Peter Maydell <peter.maydell@linaro.org>
Date: Fri, 23 Mar 2018 18:26:46 +0000
Subject: [PATCH 07/10] target/arm: Honour MDCR_EL2.TDE when routing exceptions
due to BKPT/BRK
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The MDCR_EL2.TDE bit allows the exception level targeted by debug
exceptions to be set to EL2 for code executing at EL0. We handle
this in the arm_debug_target_el() function, but this is only used for
hardware breakpoint and watchpoint exceptions, not for the exception
generated when the guest executes an AArch32 BKPT or AArch64 BRK
instruction. We don't have enough information for a translate-time
equivalent of arm_debug_target_el(), so instead make BKPT and BRK
call a special purpose helper which can do the routing, rather than
the generic exception_with_syndrome helper.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180320134114.30418-2-peter.maydell@linaro.org
---
target/arm/helper.h | 1 +
target/arm/op_helper.c | 8 ++++++++
target/arm/translate-a64.c | 15 +++++++++++++--
target/arm/translate.c | 19 ++++++++++++++-----
4 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 0d2094f2be..34e8cc8904 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -47,6 +47,7 @@ DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE,
i32, i32, i32, i32)
DEF_HELPER_2(exception_internal, void, env, i32)
DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32)
+DEF_HELPER_2(exception_bkpt_insn, void, env, i32)
DEF_HELPER_1(setend, void, env)
DEF_HELPER_2(wfi, void, env, i32)
DEF_HELPER_1(wfe, void, env)
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 7a88fd2c92..4b123d2bd6 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -483,6 +483,14 @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
raise_exception(env, excp, syndrome, target_el);
}
+/* Raise an EXCP_BKPT with the specified syndrome register value,
+ * targeting the correct exception level for debug exceptions.
+ */
+void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
+{
+ raise_exception(env, EXCP_BKPT, syndrome, arm_debug_target_el(env));
+}
+
uint32_t HELPER(cpsr_read)(CPUARMState *env)
{
return cpsr_read(env) & ~(CPSR_EXEC | CPSR_RESERVED);
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 327513ef40..c91329249d 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -321,6 +321,18 @@ static void gen_exception_insn(DisasContext *s, int offset, int excp,
s->base.is_jmp = DISAS_NORETURN;
}
+static void gen_exception_bkpt_insn(DisasContext *s, int offset,
+ uint32_t syndrome)
+{
+ TCGv_i32 tcg_syn;
+
+ gen_a64_set_pc_im(s->pc - offset);
+ tcg_syn = tcg_const_i32(syndrome);
+ gen_helper_exception_bkpt_insn(cpu_env, tcg_syn);
+ tcg_temp_free_i32(tcg_syn);
+ s->base.is_jmp = DISAS_NORETURN;
+}
+
static void gen_ss_advance(DisasContext *s)
{
/* If the singlestep state is Active-not-pending, advance to
@@ -1839,8 +1851,7 @@ static void disas_exc(DisasContext *s, uint32_t insn)
break;
}
/* BRK */
- gen_exception_insn(s, 4, EXCP_BKPT, syn_aa64_bkpt(imm16),
- default_exception_el(s));
+ gen_exception_bkpt_insn(s, 4, syn_aa64_bkpt(imm16));
break;
case 2:
if (op2_ll != 0) {
diff --git a/target/arm/translate.c b/target/arm/translate.c
index ba6ab7d287..fc03b5b8c8 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1248,6 +1248,18 @@ static void gen_exception_insn(DisasContext *s, int offset, int excp,
s->base.is_jmp = DISAS_NORETURN;
}
+static void gen_exception_bkpt_insn(DisasContext *s, int offset, uint32_t syn)
+{
+ TCGv_i32 tcg_syn;
+
+ gen_set_condexec(s);
+ gen_set_pc_im(s, s->pc - offset);
+ tcg_syn = tcg_const_i32(syn);
+ gen_helper_exception_bkpt_insn(cpu_env, tcg_syn);
+ tcg_temp_free_i32(tcg_syn);
+ s->base.is_jmp = DISAS_NORETURN;
+}
+
/* Force a TB lookup after an instruction that changes the CPU state. */
static inline void gen_lookup_tb(DisasContext *s)
{
@@ -8774,9 +8786,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
case 1:
/* bkpt */
ARCH(5);
- gen_exception_insn(s, 4, EXCP_BKPT,
- syn_aa32_bkpt(imm16, false),
- default_exception_el(s));
+ gen_exception_bkpt_insn(s, 4, syn_aa32_bkpt(imm16, false));
break;
case 2:
/* Hypervisor call (v7) */
@@ -11983,8 +11993,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn)
{
int imm8 = extract32(insn, 0, 8);
ARCH(5);
- gen_exception_insn(s, 2, EXCP_BKPT, syn_aa32_bkpt(imm8, true),
- default_exception_el(s));
+ gen_exception_bkpt_insn(s, 2, syn_aa32_bkpt(imm8, true));
break;
}
From 81621d9ab8a0f07956e67850b15eebf6d6992eec Mon Sep 17 00:00:00 2001
From: Peter Maydell <peter.maydell@linaro.org>
Date: Fri, 23 Mar 2018 18:26:46 +0000
Subject: [PATCH 08/10] target/arm: Factor out code to calculate FSR for debug
exceptions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When a debug exception is taken to AArch32, it appears as a Prefetch
Abort, and the Instruction Fault Status Register (IFSR) must be set.
The IFSR has two possible formats, depending on whether LPAE is in
use. Factor out the code in arm_debug_excp_handler() which picks
an FSR value into its own utility function, update it to use
arm_fi_to_lfsc() and arm_fi_to_sfsc() rather than hard-coded constants,
and use the correct condition to select long or short format.
In particular this fixes a bug where we could select the short
format because we're at EL0 and the EL1 translation regime is
not using LPAE, but then route the debug exception to EL2 because
of MDCR_EL2.TDE and hand EL2 the wrong format FSR.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180320134114.30418-3-peter.maydell@linaro.org
---
target/arm/internals.h | 25 +++++++++++++++++++++++++
target/arm/op_helper.c | 12 ++----------
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 47cc224a46..8ce944b7a0 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -763,4 +763,29 @@ static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
}
}
+/* Return the FSR value for a debug exception (watchpoint, hardware
+ * breakpoint or BKPT insn) targeting the specified exception level.
+ */
+static inline uint32_t arm_debug_exception_fsr(CPUARMState *env)
+{
+ ARMMMUFaultInfo fi = { .type = ARMFault_Debug };
+ int target_el = arm_debug_target_el(env);
+ bool using_lpae = false;
+
+ if (target_el == 2 || arm_el_is_aa64(env, target_el)) {
+ using_lpae = true;
+ } else {
+ if (arm_feature(env, ARM_FEATURE_LPAE) &&
+ (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) {
+ using_lpae = true;
+ }
+ }
+
+ if (using_lpae) {
+ return arm_fi_to_lfsc(&fi);
+ } else {
+ return arm_fi_to_sfsc(&fi);
+ }
+}
+
#endif
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 4b123d2bd6..75efff9edf 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -1330,11 +1330,7 @@ void arm_debug_excp_handler(CPUState *cs)
cs->watchpoint_hit = NULL;
- if (extended_addresses_enabled(env)) {
- env->exception.fsr = (1 << 9) | 0x22;
- } else {
- env->exception.fsr = 0x2;
- }
+ env->exception.fsr = arm_debug_exception_fsr(env);
env->exception.vaddress = wp_hit->hitaddr;
raise_exception(env, EXCP_DATA_ABORT,
syn_watchpoint(same_el, 0, wnr),
@@ -1354,11 +1350,7 @@ void arm_debug_excp_handler(CPUState *cs)
return;
}
- if (extended_addresses_enabled(env)) {
- env->exception.fsr = (1 << 9) | 0x22;
- } else {
- env->exception.fsr = 0x2;
- }
+ env->exception.fsr = arm_debug_exception_fsr(env);
/* FAR is UNKNOWN, so doesn't need setting */
raise_exception(env, EXCP_PREFETCH_ABORT,
syn_breakpoint(same_el),
From 62b94f31d0df75187bb00684fc29e8639eacc0c5 Mon Sep 17 00:00:00 2001
From: Peter Maydell <peter.maydell@linaro.org>
Date: Fri, 23 Mar 2018 18:26:46 +0000
Subject: [PATCH 09/10] target/arm: Set FSR for BKPT, BRK when raising
exception
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Now that we have a helper function specifically for the BRK and
BKPT instructions, we can set the exception.fsr there rather
than in arm_cpu_do_interrupt_aarch32(). This allows us to
use our new arm_debug_exception_fsr() helper.
In particular this fixes a bug where we were hardcoding the
short-form IFSR value, which is wrong if the target exception
level has LPAE enabled.
Fixes: https://bugs.launchpad.net/qemu/+bug/1756927
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180320134114.30418-4-peter.maydell@linaro.org
---
target/arm/helper.c | 1 -
target/arm/op_helper.c | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 09893e3f72..dcb8476d9e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7910,7 +7910,6 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
offset = 0;
break;
case EXCP_BKPT:
- env->exception.fsr = 2;
/* Fall through to prefetch abort. */
case EXCP_PREFETCH_ABORT:
A32_BANKED_CURRENT_REG_SET(env, ifsr, env->exception.fsr);
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 75efff9edf..8e1e521193 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -488,6 +488,8 @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
*/
void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
{
+ /* FSR will only be used if the debug target EL is AArch32. */
+ env->exception.fsr = arm_debug_exception_fsr(env);
raise_exception(env, EXCP_BKPT, syndrome, arm_debug_target_el(env));
}
From 548f514cf89dd9ab39c0cb4c063097bccf141fdd Mon Sep 17 00:00:00 2001
From: Peter Maydell <peter.maydell@linaro.org>
Date: Fri, 23 Mar 2018 18:26:46 +0000
Subject: [PATCH 10/10] target/arm: Always set FAR to a known unknown value for
debug exceptions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
For debug exceptions due to breakpoints or the BKPT instruction which
are taken to AArch32, the Fault Address Register is architecturally
UNKNOWN. We were using that as license to simply not set
env->exception.vaddress, but this isn't correct, because it will
expose to the guest whatever old value was in that field when
arm_cpu_do_interrupt_aarch32() writes it to the guest IFSR. That old
value might be a FAR for a previous guest EL2 or secure exception, in
which case we shouldn't show it to an EL1 or non-secure exception
handler. It might also be a non-deterministic value, which is bad
for record-and-replay.
Clear env->exception.vaddress before taking breakpoint debug
exceptions, to avoid this minor information leak.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180320134114.30418-5-peter.maydell@linaro.org
---
target/arm/op_helper.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 8e1e521193..a266cc0116 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -490,6 +490,11 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
{
/* FSR will only be used if the debug target EL is AArch32. */
env->exception.fsr = arm_debug_exception_fsr(env);
+ /* FAR is UNKNOWN: clear vaddress to avoid potentially exposing
+ * values to the guest that it shouldn't be able to see at its
+ * exception/security level.
+ */
+ env->exception.vaddress = 0;
raise_exception(env, EXCP_BKPT, syndrome, arm_debug_target_el(env));
}
@@ -1353,7 +1358,11 @@ void arm_debug_excp_handler(CPUState *cs)
}
env->exception.fsr = arm_debug_exception_fsr(env);
- /* FAR is UNKNOWN, so doesn't need setting */
+ /* FAR is UNKNOWN: clear vaddress to avoid potentially exposing
+ * values to the guest that it shouldn't be able to see at its
+ * exception/security level.
+ */
+ env->exception.vaddress = 0;
raise_exception(env, EXCP_PREFETCH_ABORT,
syn_breakpoint(same_el),
arm_debug_target_el(env));