Ten arm-related bug fixes for 2.12...

thanks

-- PMM

The following changes since commit 4c2c1015905fa1d616750dfe024b4c0b35875950:

Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20180323' into staging (2018-03-23 10:20:54 +0000)

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

for you to fetch changes up to 548f514cf89dd9ab39c0cb4c063097bccf141fdd:

target/arm: Always set FAR to a known unknown value for debug exceptions (2018-03-23 18:26:46 +0000)

* arm/translate-a64: don't lose interrupts after unmasking via write to DAIF

* sdhci: fix incorrect use of Error *

* hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses

* hw/arm/bcm2836: Use the Cortex-A7 instead of Cortex-A15

* i.MX: Support serial RS-232 break properly

* mach-virt: Set VM's SMBIOS system version to mc->name

* target/arm: Honour MDCR_EL2.TDE when routing exceptions due to BKPT/BRK

* target/arm: Factor out code to calculate FSR for debug exceptions

* target/arm: Set FSR for BKPT, BRK when raising exception

* target/arm: Always set FAR to a known unknown value for debug exceptions

Paolo Bonzini (1):

sdhci: fix incorrect use of Error *

Peter Maydell (6):

hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses

hw/arm/bcm2836: Use the Cortex-A7 instead of Cortex-A15

target/arm: Honour MDCR_EL2.TDE when routing exceptions due to BKPT/BRK

target/arm: Factor out code to calculate FSR for debug exceptions

target/arm: Set FSR for BKPT, BRK when raising exception

target/arm: Always set FAR to a known unknown value for debug exceptions

Trent Piepho (1):

i.MX: Support serial RS-232 break properly

Victor Kamensky (1):

arm/translate-a64: treat DISAS_UPDATE as variant of DISAS_EXIT

Wei Huang (1):

mach-virt: Set VM's SMBIOS system version to mc->name

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.

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

--- a/target/arm/op_helper.c

+++ b/target/arm/op_helper.c

@@ -XXX,XX +XXX,XX @@ 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));

@@ -XXX,XX +XXX,XX @@ 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));

2.16.2

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.

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

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ 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

--- a/target/arm/op_helper.c

+++ b/target/arm/op_helper.c

@@ -XXX,XX +XXX,XX @@ 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),

@@ -XXX,XX +XXX,XX @@ 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),

2.16.2

From: Wei Huang <wei@redhat.com>

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

include/hw/arm/virt.h | 1 +

hw/arm/virt.c | 8 +++++++-

2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

--- a/include/hw/arm/virt.h

+++ b/include/hw/arm/virt.h

@@ -XXX,XX +XXX,XX @@ typedef struct {

bool no_its;

bool no_pmu;

bool claim_edge_triggered_timers;

+ bool smbios_old_sys_ver;

} VirtMachineClass;

typedef struct {

@@ -XXX,XX +XXX,XX @@ 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";

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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)

2.16.2

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

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

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/target/arm/op_helper.c

+++ b/target/arm/op_helper.c

@@ -XXX,XX +XXX,XX @@ 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));

}

2.16.2

From: Paolo Bonzini <pbonzini@redhat.com>

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>

hw/sd/sdhci.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c

--- a/hw/sd/sdhci.c

+++ b/hw/sd/sdhci.c

@@ -XXX,XX +XXX,XX @@ 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;

@@ -XXX,XX +XXX,XX @@ 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;

2.16.2

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

--- a/hw/intc/arm_gicv3_cpuif.c

+++ b/hw/intc/arm_gicv3_cpuif.c

@@ -XXX,XX +XXX,XX @@ 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) {

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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) {

2.16.2

From: Trent Piepho <tpiepho@impinj.com>

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>

include/hw/char/imx_serial.h | 1 +

hw/char/imx_serial.c | 5 ++++-

2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h

--- a/include/hw/char/imx_serial.h

+++ b/include/hw/char/imx_serial.h

#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 */

diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/char/imx_serial.c

+++ b/hw/char/imx_serial.c

@@ -XXX,XX +XXX,XX @@ 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);

}

@@ -XXX,XX +XXX,XX @@ 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);

2.16.2

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.

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

--- a/target/arm/helper.h

+++ b/target/arm/helper.h

@@ -XXX,XX +XXX,XX @@ 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