Target-arm queue for rc2 -- just some minor bugfixes.

The following changes since commit 6e5d4999c761ffa082f60d72a14e5c953515b417:

Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2019-11-19' into staging (2019-11-19 11:29:01 +0000)

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

for you to fetch changes up to 04c9c81b8fa2ee33f59a26265700fae6fc646062:

target/arm: Support EL0 v7m msr/mrs for CONFIG_USER_ONLY (2019-11-19 13:20:28 +0000)

* Support EL0 v7m msr/mrs for CONFIG_USER_ONLY

* Relax r13 restriction for ldrex/strex for v8.0

* Do not reject rt == rt2 for strexd

* net/cadence_gem: Set PHY autonegotiation restart status

* ssi: xilinx_spips: Skip spi bus update for a few register writes

* pl031: Expose RTCICR as proper WC register

Alexander Graf (1):

pl031: Expose RTCICR as proper WC register

Linus Ziegert (1):

net/cadence_gem: Set PHY autonegotiation restart status

Richard Henderson (4):

target/arm: Merge arm_cpu_vq_map_next_smaller into sole caller

target/arm: Do not reject rt == rt2 for strexd

target/arm: Relax r13 restriction for ldrex/strex for v8.0

target/arm: Support EL0 v7m msr/mrs for CONFIG_USER_ONLY

Sai Pavan Boddu (1):

ssi: xilinx_spips: Skip spi bus update for a few register writes

target/arm/cpu.h | 5 +--

hw/net/cadence_gem.c | 9 ++--

hw/rtc/pl031.c | 6 +--

hw/ssi/xilinx_spips.c | 22 ++++++++--

target/arm/cpu64.c | 15 -------

target/arm/helper.c | 9 +++-

target/arm/m_helper.c | 114 ++++++++++++++++++++++++++++++-------------------

target/arm/translate.c | 14 +++---

8 files changed, 113 insertions(+), 81 deletions(-)

From: Richard Henderson <richard.henderson@linaro.org>

There was too much cut and paste between ldrexd and strexd,

as ldrexd does prohibit two output registers the same.

Fixes: af288228995

Reported-by: Michael Goffioul <michael.goffioul@gmail.com>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20191117090621.32425-2-richard.henderson@linaro.org

Reviewed-by: Robert Foley <robert.foley@linaro.org>

target/arm/translate.c | 2 +-

diff --git a/target/arm/translate.c b/target/arm/translate.c

--- a/target/arm/translate.c

+++ b/target/arm/translate.c

@@ -XXX,XX +XXX,XX @@ static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel)

|| (s->thumb && (a->rd == 13 || a->rt == 13))

|| (mop == MO_64

&& (a->rt2 == 15

- || a->rd == a->rt2 || a->rt == a->rt2

+ || a->rd == a->rt2

|| (s->thumb && a->rt2 == 13)))) {

unallocated_encoding(s);

return true;

From: Richard Henderson <richard.henderson@linaro.org>

Simply moving the non-stub helper_v7m_mrs/msr outside of

!CONFIG_USER_ONLY is not an option, because of all of the

other system-mode helpers that are called.

But we can split out a few subroutines to handle the few

EL0 accessible registers without duplicating code.

Reported-by: Christophe Lyon <christophe.lyon@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20191118194916.3670-1-richard.henderson@linaro.org

[PMM: deleted now-redundant comment; added a default case

to switch in v7m_msr helper]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

target/arm/cpu.h | 2 +

target/arm/m_helper.c | 114 ++++++++++++++++++++++++++----------------

2 files changed, 73 insertions(+), 43 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)

if (mask & XPSR_GE) {

env->GE = (val & XPSR_GE) >> 16;

@@ -XXX,XX +XXX,XX @@

#include "exec/cpu_ldst.h"

#endif

+static void v7m_msr_xpsr(CPUARMState *env, uint32_t mask,

+ uint32_t reg, uint32_t val)

+{

+ /* Only APSR is actually writable */

+ if (!(reg & 4)) {

+ uint32_t apsrmask = 0;

+ if (mask & 8) {

+ apsrmask |= XPSR_NZCV | XPSR_Q;

+ }

+ if ((mask & 4) && arm_feature(env, ARM_FEATURE_THUMB_DSP)) {

+ apsrmask |= XPSR_GE;

+ }

+ xpsr_write(env, val, apsrmask);

+ }

+}

+

+static uint32_t v7m_mrs_xpsr(CPUARMState *env, uint32_t reg, unsigned el)

+{

+ uint32_t mask = 0;

+

+ if ((reg & 1) && el) {

+ mask |= XPSR_EXCP; /* IPSR (unpriv. reads as zero) */

+ }

+ if (!(reg & 4)) {

+ mask |= XPSR_NZCV | XPSR_Q; /* APSR */

+ if (arm_feature(env, ARM_FEATURE_THUMB_DSP)) {

+ mask |= XPSR_GE;

+ }

+ }

+ /* EPSR reads as zero */

+ return xpsr_read(env) & mask;

+}

+

+static uint32_t v7m_mrs_control(CPUARMState *env, uint32_t secure)

+{

+ uint32_t value = env->v7m.control[secure];

+

+ if (!secure) {

+ /* SFPA is RAZ/WI from NS; FPCA is stored in the M_REG_S bank */

+ value |= env->v7m.control[M_REG_S] & R_V7M_CONTROL_FPCA_MASK;

+ }

+ return value;

+}

+

#ifdef CONFIG_USER_ONLY

-/* These should probably raise undefined insn exceptions. */

-void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)

+void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)

{

- ARMCPU *cpu = env_archcpu(env);

+ uint32_t mask = extract32(maskreg, 8, 4);

+ uint32_t reg = extract32(maskreg, 0, 8);

- cpu_abort(CPU(cpu), "v7m_msr %d\n", reg);

+ switch (reg) {

+ case 0 ... 7: /* xPSR sub-fields */

+ v7m_msr_xpsr(env, mask, reg, val);

+ break;

+ case 20: /* CONTROL */

+ /* There are no sub-fields that are actually writable from EL0. */

+ break;

+ default:

+ /* Unprivileged writes to other registers are ignored */

+ break;

+ }

}

uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)

{

- ARMCPU *cpu = env_archcpu(env);

-

- cpu_abort(CPU(cpu), "v7m_mrs %d\n", reg);

- return 0;

+ switch (reg) {

+ case 0 ... 7: /* xPSR sub-fields */

+ return v7m_mrs_xpsr(env, reg, 0);

+ case 20: /* CONTROL */

+ return v7m_mrs_control(env, 0);

+ default:

+ /* Unprivileged reads others as zero. */

+ return 0;

+ }

}

void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest)

@@ -XXX,XX +XXX,XX @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)

uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)

{

- uint32_t mask;

unsigned el = arm_current_el(env);

/* First handle registers which unprivileged can read */

-

switch (reg) {

case 0 ... 7: /* xPSR sub-fields */

- mask = 0;

- if ((reg & 1) && el) {

- mask |= XPSR_EXCP; /* IPSR (unpriv. reads as zero) */

- }

- if (!(reg & 4)) {

- mask |= XPSR_NZCV | XPSR_Q; /* APSR */

- if (arm_feature(env, ARM_FEATURE_THUMB_DSP)) {

- mask |= XPSR_GE;

- }

- }

- /* EPSR reads as zero */

- return xpsr_read(env) & mask;

- break;

+ return v7m_mrs_xpsr(env, reg, el);

case 20: /* CONTROL */

- {

- uint32_t value = env->v7m.control[env->v7m.secure];

- if (!env->v7m.secure) {

- /* SFPA is RAZ/WI from NS; FPCA is stored in the M_REG_S bank */

- value |= env->v7m.control[M_REG_S] & R_V7M_CONTROL_FPCA_MASK;

- }

- return value;

- }

+ return v7m_mrs_control(env, env->v7m.secure);

case 0x94: /* CONTROL_NS */

/*

* We have to handle this here because unprivileged Secure code

Armv8-A removes UNPREDICTABLE for R13 for these cases.

target/arm/translate.c | 12 ++++++++----

1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c

--- a/target/arm/translate.c

+++ b/target/arm/translate.c

@@ -XXX,XX +XXX,XX @@ static bool trans_SWPB(DisasContext *s, arg_SWP *a)

static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel)

TCGv_i32 addr;

+ /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */

+ bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M);

/* We UNDEF for these UNPREDICTABLE cases. */

if (a->rd == 15 || a->rn == 15 || a->rt == 15

|| a->rd == a->rn || a->rd == a->rt

- || (s->thumb && (a->rd == 13 || a->rt == 13))

+ || (!v8a && s->thumb && (a->rd == 13 || a->rt == 13))

|| (mop == MO_64

&& (a->rt2 == 15

|| a->rd == a->rt2

- || (s->thumb && a->rt2 == 13)))) {

+ || (!v8a && s->thumb && a->rt2 == 13)))) {

unallocated_encoding(s);

return true;

}

@@ -XXX,XX +XXX,XX @@ static bool trans_STLH(DisasContext *s, arg_STL *a)

static bool op_ldrex(DisasContext *s, arg_LDREX *a, MemOp mop, bool acq)

{

TCGv_i32 addr;

+ /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */

+ bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M);

/* We UNDEF for these UNPREDICTABLE cases. */

if (a->rn == 15 || a->rt == 15

- || (s->thumb && a->rt == 13)

+ || (!v8a && s->thumb && a->rt == 13)

|| (mop == MO_64

&& (a->rt2 == 15 || a->rt == a->rt2

- || (s->thumb && a->rt2 == 13)))) {

+ || (!v8a && s->thumb && a->rt2 == 13)))) {

unallocated_encoding(s);

return true;

}

From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

A few configuration register writes need not update the spi bus state, so just

return after the register write.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

Tested-by: Francisco Iglesias <frasse.iglesias@gmail.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Message-id: 1573830705-14579-1-git-send-email-sai.pavan.boddu@xilinx.com

hw/ssi/xilinx_spips.c | 22 ++++++++++++++++++----

1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c

--- a/hw/ssi/xilinx_spips.c

+++ b/hw/ssi/xilinx_spips.c

@@ -XXX,XX +XXX,XX @@

#define R_GPIO (0x30 / 4)

#define R_LPBK_DLY_ADJ (0x38 / 4)

#define R_LPBK_DLY_ADJ_RESET (0x33)

+#define R_IOU_TAPDLY_BYPASS (0x3C / 4)

#define R_TXD1 (0x80 / 4)

#define R_TXD2 (0x84 / 4)

#define R_TXD3 (0x88 / 4)

@@ -XXX,XX +XXX,XX @@

#define R_LQSPI_STS (0xA4 / 4)

#define LQSPI_STS_WR_RECVD (1 << 1)

+#define R_DUMMY_CYCLE_EN (0xC8 / 4)

+#define R_ECO (0xF8 / 4)

#define R_MOD_ID (0xFC / 4)

#define R_GQSPI_SELECT (0x144 / 4)

@@ -XXX,XX +XXX,XX @@ static void xilinx_spips_write(void *opaque, hwaddr addr,

int mask = ~0;

XilinxSPIPS *s = opaque;

+ bool try_flush = true;

DB_PRINT_L(0, "addr=" TARGET_FMT_plx " = %x\n", addr, (unsigned)value);

addr >>= 2;

@@ -XXX,XX +XXX,XX @@ static void xilinx_spips_write(void *opaque, hwaddr addr,

tx_data_bytes(&s->tx_fifo, (uint32_t)value, 3,

s->regs[R_CONFIG] & R_CONFIG_ENDIAN);

goto no_reg_update;

+ /* Skip SPI bus update for below registers writes */

+ case R_GPIO:

+ case R_LPBK_DLY_ADJ:

+ case R_IOU_TAPDLY_BYPASS:

+ case R_DUMMY_CYCLE_EN:

+ case R_ECO:

+ try_flush = false;

+ break;

s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);

no_reg_update:

- xilinx_spips_update_cs_lines(s);

- xilinx_spips_check_flush(s);

- xilinx_spips_update_cs_lines(s);

- xilinx_spips_update_ixr(s);

+ if (try_flush) {

+ xilinx_spips_update_cs_lines(s);

+ xilinx_spips_check_flush(s);

+ xilinx_spips_update_cs_lines(s);

+ xilinx_spips_update_ixr(s);

+ }

static const MemoryRegionOps spips_ops = {

Coverity reports, in sve_zcr_get_valid_len,

"Subtract operation overflows on operands

arm_cpu_vq_map_next_smaller(cpu, start_vq + 1U) and 1U"

First, the aarch32 stub version of arm_cpu_vq_map_next_smaller,

returning 0, does exactly what Coverity reports. Remove it.

Second, the aarch64 version of arm_cpu_vq_map_next_smaller has

a set of asserts, but they don't cover the case in question.

Further, there is a fair amount of extra arithmetic needed to

convert from the 0-based zcr register, to the 1-base vq form,

to the 0-based bitmap, and back again. This can be simplified

by leaving the value in the 0-based form.

Finally, use test_bit to simplify the common case, where the

length in the zcr registers is in fact a supported length.

Reported-by: Coverity (CID 1407217)

Reviewed-by: Andrew Jones <drjones@redhat.com>

Message-id: 20191118091414.19440-1-richard.henderson@linaro.org

target/arm/cpu.h | 3 ---

target/arm/cpu64.c | 15 ---------------

target/arm/helper.c | 9 +++++++--

3 files changed, 7 insertions(+), 20 deletions(-)

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

#ifdef TARGET_AARCH64

# define ARM_MAX_VQ 16

void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);

-uint32_t arm_cpu_vq_map_next_smaller(ARMCPU *cpu, uint32_t vq);

#else

# define ARM_MAX_VQ 1

static inline void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp) { }

-static inline uint32_t arm_cpu_vq_map_next_smaller(ARMCPU *cpu, uint32_t vq)

-{ return 0; }

#endif

typedef struct ARMVectorReg {

@@ -XXX,XX +XXX,XX @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)

cpu->sve_max_vq = max_vq;

-uint32_t arm_cpu_vq_map_next_smaller(ARMCPU *cpu, uint32_t vq)

-{

- uint32_t bitnum;

-

- /*

- * We allow vq == ARM_MAX_VQ + 1 to be input because the caller may want

- * to find the maximum vq enabled, which may be ARM_MAX_VQ, but this

- * function always returns the next smaller than the input.

- */

- assert(vq && vq <= ARM_MAX_VQ + 1);

-

- bitnum = find_last_bit(cpu->sve_vq_map, vq - 1);

- return bitnum == vq - 1 ? 0 : bitnum + 1;

-}

-

static void cpu_max_get_sve_max_vq(Object *obj, Visitor *v, const char *name,

void *opaque, Error **errp)

diff --git a/target/arm/helper.c b/target/arm/helper.c

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ int sve_exception_el(CPUARMState *env, int el)

static uint32_t sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len)

{

- uint32_t start_vq = (start_len & 0xf) + 1;

+ uint32_t end_len;

- return arm_cpu_vq_map_next_smaller(cpu, start_vq + 1) - 1;

+ end_len = start_len &= 0xf;

+ if (!test_bit(start_len, cpu->sve_vq_map)) {

+ end_len = find_last_bit(cpu->sve_vq_map, start_len);

+ assert(end_len < start_len);

+ }

+ return end_len;

/*

From: Linus Ziegert <linus.ziegert+qemu@holoplot.com>

The Linux kernel PHY driver sets AN_RESTART in the BMCR of the

PHY when autonegotiation is started.

Recently the kernel started to read back the PHY's AN_RESTART

bit and now checks whether the autonegotiation is complete and

the bit was cleared [1]. Otherwise the link status is down.

The emulated PHY needs to clear AN_RESTART immediately to inform

the kernel driver about the completion of autonegotiation phase.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c36757eb9dee

Signed-off-by: Linus Ziegert <linus.ziegert+qemu@holoplot.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Message-id: 20191104181604.21943-1-linus.ziegert+qemu@holoplot.com

Cc: qemu-stable@nongnu.org

hw/net/cadence_gem.c | 9 +++++----

1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c

--- a/hw/net/cadence_gem.c

+++ b/hw/net/cadence_gem.c

@@ -XXX,XX +XXX,XX @@

#define PHY_REG_EXT_PHYSPCFC_ST 27

#define PHY_REG_CABLE_DIAG 28

-#define PHY_REG_CONTROL_RST 0x8000

-#define PHY_REG_CONTROL_LOOP 0x4000

-#define PHY_REG_CONTROL_ANEG 0x1000

+#define PHY_REG_CONTROL_RST 0x8000

+#define PHY_REG_CONTROL_LOOP 0x4000

+#define PHY_REG_CONTROL_ANEG 0x1000

+#define PHY_REG_CONTROL_ANRESTART 0x0200

#define PHY_REG_STATUS_LINK 0x0004

#define PHY_REG_STATUS_ANEGCMPL 0x0020

@@ -XXX,XX +XXX,XX @@ static void gem_phy_write(CadenceGEMState *s, unsigned reg_num, uint16_t val)

}

if (val & PHY_REG_CONTROL_ANEG) {

/* Complete autonegotiation immediately */

- val &= ~PHY_REG_CONTROL_ANEG;

+ val &= ~(PHY_REG_CONTROL_ANEG | PHY_REG_CONTROL_ANRESTART);

s->phy_regs[PHY_REG_STATUS] |= PHY_REG_STATUS_ANEGCMPL;

}

if (val & PHY_REG_CONTROL_LOOP) {

From: Alexander Graf <graf@amazon.com>

The current PL031 RTCICR register implementation always clears the

IRQ pending status on a register write, regardless of the value the

guest writes.

To justify that behavior, it references the ARM926EJ-S Development

Chip Reference Manual (DDI0287B) and indicates that said document

states that any write clears the internal IRQ state. It is indeed

true that in section 11.1 this document says:

"The interrupt is cleared by writing any data value to the

interrupt clear register RTCICR".

However, later in section 11.2.2 it contradicts itself by saying:

"Writing 1 to bit 0 of RTCICR clears the RTCINTR flag."

The latter statement matches the PL031 TRM (DDI0224C), which says:

"Writing 1 to bit position 0 clears the corresponding interrupt.

Writing 0 has no effect."

Let's assume that the self-contradictory DDI0287B is in error, and

follow the reference manual for the device itself, by making the

register write-one-to-clear.

Reported-by: Hendrik Borghorst <hborghor@amazon.de>

Signed-off-by: Alexander Graf <graf@amazon.com>

Message-id: 20191104115228.30745-1-graf@amazon.com

[PMM: updated commit message to note that DDI0287B says two

conflicting things]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

hw/rtc/pl031.c | 6 +-----

1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/rtc/pl031.c b/hw/rtc/pl031.c

--- a/hw/rtc/pl031.c

+++ b/hw/rtc/pl031.c

@@ -XXX,XX +XXX,XX @@ static void pl031_write(void * opaque, hwaddr offset,

pl031_update(s);

break;

case RTC_ICR:

- /* The PL031 documentation (DDI0224B) states that the interrupt is

- cleared when bit 0 of the written value is set. However the

- arm926e documentation (DDI0287B) states that the interrupt is

- cleared when any value is written. */

- s->is = 0;

+ s->is &= ~value;

pl031_update(s);

break;

case RTC_CR: