All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/7] target-arm queue
@ 2019-11-19 13:31 Peter Maydell
  2019-11-19 13:31 ` [PULL 1/7] pl031: Expose RTCICR as proper WC register Peter Maydell
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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

thanks
-- PMM

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)

are available in the Git repository at:

  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)

----------------------------------------------------------------
target-arm queue:
 * 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(-)


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

* [PULL 1/7] pl031: Expose RTCICR as proper WC register
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
@ 2019-11-19 13:31 ` Peter Maydell
  2019-11-19 13:31 ` [PULL 2/7] target/arm: Merge arm_cpu_vq_map_next_smaller into sole caller Peter Maydell
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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>
Signed-off-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
index 3a982752a2a..c57cf83165d 100644
--- a/hw/rtc/pl031.c
+++ b/hw/rtc/pl031.c
@@ -149,11 +149,7 @@ 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:
-- 
2.20.1



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

* [PULL 2/7] target/arm: Merge arm_cpu_vq_map_next_smaller into sole caller
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
  2019-11-19 13:31 ` [PULL 1/7] pl031: Expose RTCICR as proper WC register Peter Maydell
@ 2019-11-19 13:31 ` Peter Maydell
  2019-11-19 13:31 ` [PULL 3/7] ssi: xilinx_spips: Skip spi bus update for a few register writes Peter Maydell
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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

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)
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20191118091414.19440-1-richard.henderson@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h    |  3 ---
 target/arm/cpu64.c  | 15 ---------------
 target/arm/helper.c |  9 +++++++--
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e1a66a2d1cc..47d24a53758 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -185,12 +185,9 @@ 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 {
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 68baf0482ff..a39d6fcea34 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -458,21 +458,6 @@ 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 be67e2c66d6..a089fb5a690 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5363,9 +5363,14 @@ 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;
 }
 
 /*
-- 
2.20.1



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

* [PULL 3/7] ssi: xilinx_spips: Skip spi bus update for a few register writes
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
  2019-11-19 13:31 ` [PULL 1/7] pl031: Expose RTCICR as proper WC register Peter Maydell
  2019-11-19 13:31 ` [PULL 2/7] target/arm: Merge arm_cpu_vq_map_next_smaller into sole caller Peter Maydell
@ 2019-11-19 13:31 ` Peter Maydell
  2019-11-19 13:31 ` [PULL 4/7] net/cadence_gem: Set PHY autonegotiation restart status Peter Maydell
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 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
index a309c712ca8..0d6c2e1a61d 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -109,6 +109,7 @@
 #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)
@@ -139,6 +140,8 @@
 #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)
@@ -970,6 +973,7 @@ 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;
@@ -1019,13 +1023,23 @@ 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 = {
-- 
2.20.1



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

* [PULL 4/7] net/cadence_gem: Set PHY autonegotiation restart status
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
                   ` (2 preceding siblings ...)
  2019-11-19 13:31 ` [PULL 3/7] ssi: xilinx_spips: Skip spi bus update for a few register writes Peter Maydell
@ 2019-11-19 13:31 ` Peter Maydell
  2019-11-19 13:31 ` [PULL 5/7] target/arm: Do not reject rt == rt2 for strexd Peter Maydell
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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
Signed-off-by: Peter Maydell <peter.maydell@linaro.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
index 7f9cb5ab955..b8be73dc558 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -271,9 +271,10 @@
 #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
@@ -1345,7 +1346,7 @@ 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) {
-- 
2.20.1



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

* [PULL 5/7] target/arm: Do not reject rt == rt2 for strexd
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
                   ` (3 preceding siblings ...)
  2019-11-19 13:31 ` [PULL 4/7] net/cadence_gem: Set PHY autonegotiation restart status Peter Maydell
@ 2019-11-19 13:31 ` Peter Maydell
  2019-11-19 13:31 ` [PULL 6/7] target/arm: Relax r13 restriction for ldrex/strex for v8.0 Peter Maydell
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 2ea9da7637b..b285b23858e 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8934,7 +8934,7 @@ 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;
-- 
2.20.1



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

* [PULL 6/7] target/arm: Relax r13 restriction for ldrex/strex for v8.0
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
                   ` (4 preceding siblings ...)
  2019-11-19 13:31 ` [PULL 5/7] target/arm: Do not reject rt == rt2 for strexd Peter Maydell
@ 2019-11-19 13:31 ` Peter Maydell
  2019-11-19 13:31 ` [PULL 7/7] target/arm: Support EL0 v7m msr/mrs for CONFIG_USER_ONLY Peter Maydell
  2019-11-19 15:55 ` [PULL 0/7] target-arm queue Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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

Armv8-A removes UNPREDICTABLE for R13 for these cases.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20191117090621.32425-3-richard.henderson@linaro.org
[PMM: changed ENABLE_ARCH_8 checks to check a new bool 'v8a',
 since these cases are still UNPREDICTABLE for v8M]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/translate.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index b285b23858e..4d5d4bd8886 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8927,15 +8927,17 @@ 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;
     }
@@ -9084,13 +9086,15 @@ 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;
     }
-- 
2.20.1



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

* [PULL 7/7] target/arm: Support EL0 v7m msr/mrs for CONFIG_USER_ONLY
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
                   ` (5 preceding siblings ...)
  2019-11-19 13:31 ` [PULL 6/7] target/arm: Relax r13 restriction for ldrex/strex for v8.0 Peter Maydell
@ 2019-11-19 13:31 ` Peter Maydell
  2019-11-19 15:55 ` [PULL 0/7] target-arm queue Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 13:31 UTC (permalink / raw)
  To: qemu-devel

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>
Signed-off-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
index 47d24a53758..83a809d4bac 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1314,6 +1314,7 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
     if (mask & XPSR_GE) {
         env->GE = (val & XPSR_GE) >> 16;
     }
+#ifndef CONFIG_USER_ONLY
     if (mask & XPSR_T) {
         env->thumb = ((val & XPSR_T) != 0);
     }
@@ -1329,6 +1330,7 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
         /* Note that this only happens on exception exit */
         write_v7m_exception(env, val & XPSR_EXCP);
     }
+#endif
 }
 
 #define HCR_VM        (1ULL << 0)
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index f2512e448e2..4a48b792520 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -33,22 +33,82 @@
 #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)
@@ -2196,35 +2256,14 @@ 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
@@ -2454,18 +2493,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
 
     switch (reg) {
     case 0 ... 7: /* xPSR sub-fields */
-        /* 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);
-        }
+        v7m_msr_xpsr(env, mask, reg, val);
         break;
     case 8: /* MSP */
         if (v7m_using_psp(env)) {
-- 
2.20.1



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

* Re: [PULL 0/7] target-arm queue
  2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
                   ` (6 preceding siblings ...)
  2019-11-19 13:31 ` [PULL 7/7] target/arm: Support EL0 v7m msr/mrs for CONFIG_USER_ONLY Peter Maydell
@ 2019-11-19 15:55 ` Peter Maydell
  7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2019-11-19 15:55 UTC (permalink / raw)
  To: QEMU Developers

On Tue, 19 Nov 2019 at 13:31, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Target-arm queue for rc2 -- just some minor bugfixes.
>
> thanks
> -- PMM
>
> 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)
>
> are available in the Git repository at:
>
>   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)
>
> ----------------------------------------------------------------
> target-arm queue:
>  * 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
>



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.

-- PMM


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

end of thread, other threads:[~2019-11-19 15:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-19 13:31 [PULL 0/7] target-arm queue Peter Maydell
2019-11-19 13:31 ` [PULL 1/7] pl031: Expose RTCICR as proper WC register Peter Maydell
2019-11-19 13:31 ` [PULL 2/7] target/arm: Merge arm_cpu_vq_map_next_smaller into sole caller Peter Maydell
2019-11-19 13:31 ` [PULL 3/7] ssi: xilinx_spips: Skip spi bus update for a few register writes Peter Maydell
2019-11-19 13:31 ` [PULL 4/7] net/cadence_gem: Set PHY autonegotiation restart status Peter Maydell
2019-11-19 13:31 ` [PULL 5/7] target/arm: Do not reject rt == rt2 for strexd Peter Maydell
2019-11-19 13:31 ` [PULL 6/7] target/arm: Relax r13 restriction for ldrex/strex for v8.0 Peter Maydell
2019-11-19 13:31 ` [PULL 7/7] target/arm: Support EL0 v7m msr/mrs for CONFIG_USER_ONLY Peter Maydell
2019-11-19 15:55 ` [PULL 0/7] target-arm queue Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.