[Qemu-devel] [PATCH 0/7] target-mips: support MTTCG feature

[Qemu-devel] [PATCH 0/7] target-mips: support MTTCG feature

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@mips.com>

This series introduces MTTCG feature for MIPS targets by adding all
missing bits and pieces, and formally enabling corresponding QEMU
builds to support such configurations.

PATCH ORGANIZATION
==================

The organization of patches is as follows:

  - patches 1 and 2 deal with MIPS' LL/SC instruction emulation
    improvements related to MTTCG. They are based on a previously
    sent patch series by Leon Alrae (this is the last version, v3):
    http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg06870.html

  - patches 3, 4, 5, and 6 deal with locking/synchronization issues
    that surfaced while introducing MTTCG for MIPS. Similar sets of
    patches have been already integrated for some other platforms
    (arm, intel, ppc, sparc).

  - patch 7 just enables QEMU build system to support MTTCG feature
    for MIPS targets.

PERFORMANCE TESTING
===================

Performance testing was performed using atomic_add-bench test program
that tests LL/SC-related functionality in multithread environment. The
observed performance gain was significant.

For the sake of comparison, test case organization mimics the one from
a previously sent patch set:

target-arm: emulate aarch64's LL/SC using cmpxchg helpers
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg06653.html

-----------------------------------------------------------------------

          atomic_add-bench: 1000000 ops/thread, [0,1] range
                                               
throughput                                  M - MTTCG     N - no MTTCG

 50 +---------+---------+---------+---------+---------+---------+----+
    |                                                                |
    |M                                                               |
 40 +.                                                               +
    |.                                                               |
    |.                                                               |
 30 +.                                                               +
    |.                                                               |
    |.                                                               |
 20 +.                                                               +
    | M                                                              |
    | .                                                              |
 10 +  .M...M.......M.......M.......M.......M.......M.......M.......M+
    |N                                                               |
    | N.N...N.......N.......N.......N.......N.......N.......N.......N|
  0 +---------+---------+---------+---------+---------+---------+----+
    0         10        20        30        40        50        60

                            number of threads

-----------------------------------------------------------------------

          atomic_add-bench: 1000000 ops/thread, [0,2] range
                                               
throughput                                  M - MTTCG     N - no MTTCG

 50 +---------+---------+---------+---------+---------+---------+----+
    |                                                                |
    |M                                                               |
 40 +.                                                               +
    |.                                                               |
    |.                                                               |
 30 + .                                                              +
    | M                                                              |
    | .                                                              |
 20 +  .M...M.......M.......M.......M.......M.......M.......M.......M+
    |                                                                |
    |                                                                |
 10 +                                                                +
    |N                                                               |
    | N.N...N.......N.......N.......N.......N.......N.......N.......N|
  0 +---------+---------+---------+---------+---------+---------+----+
    0         10        20        30        40        50        60

                            number of threads

-----------------------------------------------------------------------

          atomic_add-bench: 1000000 ops/thread, [0,1] range
                                               
throughput                                  M - MTTCG     N - no MTTCG

150 +---------+---------+---------+---------+---------+---------+----+
    |                                                                |
    |                                            ...M...        ....M|
120 +                   ....M.......M........M...       ....M...     +
    |           ....M...                                             |
    |     ..M...                                                     |
 90 +    .                                                           +
    |  .M                                                            |
    | .                                                              |
 60 + M                                                              +
    |.                                                               |
    |M                                                               |
 30 +                                                                +
    |                                                                |
    |NN.N...N.......N.......N.......N.......N.......N.......N.......N|
  0 +---------+---------+---------+---------+---------+---------+----+
    0         10        20        30        40        50        60

                            number of threads

-----------------------------------------------------------------------

          atomic_add-bench: 1000000 ops/thread, [0,2] range
                                               
throughput                                  M - MTTCG     N - no MTTCG

150 +---------+---------+---------+---------+---------+---------+----+
    |                                            ...M.......M.......M|
    |                           ....M...       ..                    |
120 +           ....M.......M...        ....M..                      +
    |     ..M...                                                     |
    |   M.                                                           |
 90 +  .                                                             +
    | .                                                              |
    | .                                                              |
 60 + M                                                              +
    |.                                                               |
    |M                                                               |
 30 +                                                                +
    |                                                                |
    |NN.N...N.......N.......N.......N.......N.......N.......N.......N|
  0 +---------+---------+---------+---------+---------+---------+----+
    0         10        20        30        40        50        60

                            number of threads

-----------------------------------------------------------------------

Numerical data:

Ops
Range-->      1               2              128            1024

# of     no              no              no              no
 thr.    MTTCG  MTTCG    MTTCG  MTTCG    MTTCG  MTTCG    MTTCG  MTTCG

  1      4.95   42.61    4.94   42.27    4.89   42.24    4.85   41.81
  2      1.23   18.41    1.29   25.71    1.33   57.41    1.36   60.34
  4      0.46   11.99    0.48   19.69    0.53   78.98    0.50   95.39
  8      0.18    9.59    0.18   19.11    0.19  104.66    0.20  112.66
 16      0.11   11.19    0.12   19.12    0.12  108.29    0.13  121.90
 24      0.10   10.18    0.09   19.14    0.11  115.53    0.10  127.40
 32      0.11   11.15    0.12   19.36    0.09  120.60    0.10  131.60
 40      0.08   10.47    0.11   20.88    0.12  124.59    0.10  124.74
 48      0.12   11.78    0.13   20.09    0.11  129.24    0.11  137.19
 56      0.14   12.40    0.13   22.13    0.15  124.16    0.15  138.52
 64      0.14   11.08    0.20   21.08    0.18  131.28    0.19  144.84

-----------------------------------------------------------------------

Graphical representation:

 https://i.imgur.com/OtNLpVX.png

-----------------------------------------------------------------------

REGRESSION TESTING
==================

Regression testing was also performed. The main test bed for regression
testing was LTP test suite executed on QEMU-emulated Debian mips64
system.

Some LTP tests (getrusage04, copy_file_range01) that used to fail for
non-MTTCG systems, pass for MTTCG-enabled systems. Also, some LTP tests
(nanosleep01, poll02, pselect01) intermittently fail on both non-MTTCG
and MTTCG configurations, and therefore do not represent valid
regressions.

Emulation by itself did not appear to have any problems while executing
LTP test suite.

QEMU user mode MTTCG-enabled emulation was also tested to some extent.

Aleksandar Markovic (2):
  Revert "target/mips: hold BQL for timer interrupts"
  target/mips: introduce MTTCG-enabled builds

Goran Ferenc (1):
  target/mips: hold BQL in mips_vpe_wake()

Leon Alrae (2):
  target/mips: compare virtual addresses in LL/SC sequence
  target/mips: reimplement SC instruction and use cmpxchg

Miodrag Dinic (2):
  hw/mips_int: hold BQL for all interrupt requests
  hw/mips_cpc: kick a VP when putting it into Run state

 configure               |   3 ++
 hw/mips/mips_int.c      |  12 +++++
 hw/misc/mips_cpc.c      |  17 ++++++-
 linux-user/main.c       |  58 ------------------------
 target/mips/cpu.h       |   9 ++--
 target/mips/helper.c    |   6 +--
 target/mips/helper.h    |   2 -
 target/mips/machine.c   |   7 +--
 target/mips/op_helper.c |  74 +++++++++---------------------
 target/mips/translate.c | 118 ++++++++++++++++--------------------------------
 10 files changed, 100 insertions(+), 206 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/7] target/mips: compare virtual addresses in LL/SC sequence

[Aleksandar Markovic]

From: Leon Alrae <leon.alrae@imgtec.com>

Do only virtual addresses comaprisons in LL/SC sequence.

Until this patch, physical addresses had been compared in LL/SC
sequence. Unfortunately, that meant that, on each SC, the address
translation had to be done, which is a quite complex operation.
Getting rid of that allows throwing away SC helpers and having
common SC implementations in user and system mode, avoiding two
separate implementations selected by #ifdef CONFIG_USER_ONLY.

Given that LL/SC emulation was already very simplified (as only the
address and value were compared), using virtual addresses instead of
physical does not seem to be a violation. Correct guest software
should not rely on LL/SC if they accesses the same physical address
via different virtual addresses or if page mapping gets changed
between LL/SC due to manipulating TLB entries. MIPS Instruction Set
Manual clearly says that an RMW sequence must use the same address
in the LL and SC (virtual address, physical address, cacheability
and coherency attributes must be identical). Otherwise, the result of
the SC is not predictable. This patch takes advantage of this fact
and removes the virtual->physical address translation from SC helper.

lladdr served as Coprocessor 0 LLAddr register which captures physical
address of the most recent LL instruction, and also lladdr was used
for comparison with following SC physical address. This patch changes
the meaning of lladdr - now it will only keep the virtual address of
the most recent LL. Additionally we introduce CP0_LLAddr which is the
actual Coperocessor 0 LLAddr register that guest can access.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 target/mips/cpu.h       |  3 ++-
 target/mips/machine.c   |  7 ++++---
 target/mips/op_helper.c | 29 +++++++++++++++++------------
 target/mips/translate.c |  4 ++--
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 7f8ba5f..57ca861 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -480,10 +480,11 @@ struct CPUMIPSState {
 #define CP0C5_NFExists   0
     int32_t CP0_Config6;
     int32_t CP0_Config7;
+    uint64_t CP0_LLAddr;
     uint64_t CP0_MAAR[MIPS_MAAR_MAX];
     int32_t CP0_MAARI;
     /* XXX: Maybe make LLAddr per-TC? */
-    uint64_t lladdr;
+    target_ulong lladdr; /* LL virtual address compared against SC */
     target_ulong llval;
     target_ulong llnewval;
     target_ulong llreg;
diff --git a/target/mips/machine.c b/target/mips/machine.c
index 20100d5..155170c 100644
--- a/target/mips/machine.c
+++ b/target/mips/machine.c
@@ -212,8 +212,8 @@ const VMStateDescription vmstate_tlb = {
 
 const VMStateDescription vmstate_mips_cpu = {
     .name = "cpu",
-    .version_id = 10,
-    .minimum_version_id = 10,
+    .version_id = 11,
+    .minimum_version_id = 11,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
         /* Active TC */
@@ -283,9 +283,10 @@ const VMStateDescription vmstate_mips_cpu = {
         VMSTATE_INT32(env.CP0_Config3, MIPSCPU),
         VMSTATE_INT32(env.CP0_Config6, MIPSCPU),
         VMSTATE_INT32(env.CP0_Config7, MIPSCPU),
+        VMSTATE_UINT64(env.CP0_LLAddr, MIPSCPU),
         VMSTATE_UINT64_ARRAY(env.CP0_MAAR, MIPSCPU, MIPS_MAAR_MAX),
         VMSTATE_INT32(env.CP0_MAARI, MIPSCPU),
-        VMSTATE_UINT64(env.lladdr, MIPSCPU),
+        VMSTATE_UINTTL(env.lladdr, MIPSCPU),
         VMSTATE_UINTTL_ARRAY(env.CP0_WatchLo, MIPSCPU, 8),
         VMSTATE_INT32_ARRAY(env.CP0_WatchHi, MIPSCPU, 8),
         VMSTATE_UINTTL(env.CP0_XContext, MIPSCPU),
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index e537a8b..283669c 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -255,15 +255,15 @@ static inline hwaddr do_translate_address(CPUMIPSState *env,
                                                       target_ulong address,
                                                       int rw, uintptr_t retaddr)
 {
-    hwaddr lladdr;
+    hwaddr paddr;
     CPUState *cs = CPU(mips_env_get_cpu(env));
 
-    lladdr = cpu_mips_translate_address(env, address, rw);
+    paddr = cpu_mips_translate_address(env, address, rw);
 
-    if (lladdr == -1LL) {
+    if (paddr == -1LL) {
         cpu_loop_exit_restore(cs, retaddr);
     } else {
-        return lladdr;
+        return paddr;
     }
 }
 
@@ -274,7 +274,8 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx)  \
         env->CP0_BadVAddr = arg;                                              \
         do_raise_exception(env, EXCP_AdEL, GETPC());                          \
     }                                                                         \
-    env->lladdr = do_translate_address(env, arg, 0, GETPC());                 \
+    env->CP0_LLAddr = do_translate_address(env, arg, 0, GETPC());             \
+    env->lladdr = arg;                                                        \
     env->llval = do_##insn(env, arg, mem_idx, GETPC());                       \
     return env->llval;                                                        \
 }
@@ -294,7 +295,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,              \
         env->CP0_BadVAddr = arg2;                                             \
         do_raise_exception(env, EXCP_AdES, GETPC());                          \
     }                                                                         \
-    if (do_translate_address(env, arg2, 1, GETPC()) == env->lladdr) {         \
+    if (arg2 == env->lladdr) {                                                \
         tmp = do_##ld_insn(env, arg2, mem_idx, GETPC());                      \
         if (tmp == env->llval) {                                              \
             do_##st_insn(env, arg2, arg1, mem_idx, GETPC());                  \
@@ -873,7 +874,7 @@ target_ulong helper_mftc0_status(CPUMIPSState *env)
 
 target_ulong helper_mfc0_lladdr(CPUMIPSState *env)
 {
-    return (int32_t)(env->lladdr >> env->CP0_LLAddr_shift);
+    return (int32_t)(env->CP0_LLAddr >> env->CP0_LLAddr_shift);
 }
 
 target_ulong helper_mfc0_maar(CPUMIPSState *env)
@@ -949,7 +950,7 @@ target_ulong helper_dmfc0_tcschefback(CPUMIPSState *env)
 
 target_ulong helper_dmfc0_lladdr(CPUMIPSState *env)
 {
-    return env->lladdr >> env->CP0_LLAddr_shift;
+    return env->CP0_LLAddr >> env->CP0_LLAddr_shift;
 }
 
 target_ulong helper_dmfc0_maar(CPUMIPSState *env)
@@ -1177,7 +1178,8 @@ void helper_mtc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
 {
     env->active_tc.PC = arg1;
     env->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
-    env->lladdr = 0ULL;
+    env->CP0_LLAddr = 0;
+    env->lladdr = 0;
     /* MIPS16 not implemented. */
 }
 
@@ -1189,12 +1191,14 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
     if (other_tc == other->current_tc) {
         other->active_tc.PC = arg1;
         other->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
-        other->lladdr = 0ULL;
+        other->CP0_LLAddr = 0;
+        other->lladdr = 0;
         /* MIPS16 not implemented. */
     } else {
         other->tcs[other_tc].PC = arg1;
         other->tcs[other_tc].CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
-        other->lladdr = 0ULL;
+        other->CP0_LLAddr = 0;
+        other->lladdr = 0;
         /* MIPS16 not implemented. */
     }
 }
@@ -1620,7 +1624,7 @@ void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1)
 {
     target_long mask = env->CP0_LLAddr_rw_bitmask;
     arg1 = arg1 << env->CP0_LLAddr_shift;
-    env->lladdr = (env->lladdr & ~mask) | (arg1 & mask);
+    env->CP0_LLAddr = (env->CP0_LLAddr & ~mask) | (arg1 & mask);
 }
 
 #define MTC0_MAAR_MASK(env) \
@@ -2318,6 +2322,7 @@ static inline void exception_return(CPUMIPSState *env)
 void helper_eret(CPUMIPSState *env)
 {
     exception_return(env);
+    env->CP0_LLAddr = 1;
     env->lladdr = 1;
 }
 
diff --git a/target/mips/translate.c b/target/mips/translate.c
index d05ee67..c9104a7 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4913,7 +4913,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
     case 17:
         switch (sel) {
         case 0:
-            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, lladdr),
+            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, CP0_LLAddr),
                              ctx->CP0_LLAddr_shift);
             rn = "LLAddr";
             break;
@@ -20440,7 +20440,7 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
                 env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
     cpu_fprintf(f, "    Config0 0x%08x Config1 0x%08x LLAddr 0x%016"
                 PRIx64 "\n",
-                env->CP0_Config0, env->CP0_Config1, env->lladdr);
+                env->CP0_Config0, env->CP0_Config1, env->CP0_LLAddr);
     cpu_fprintf(f, "    Config2 0x%08x Config3 0x%08x\n",
                 env->CP0_Config2, env->CP0_Config3);
     cpu_fprintf(f, "    Config4 0x%08x Config5 0x%08x\n",
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/7] target/mips: compare virtual addresses in LL/SC sequence

[Alex Bennée]

Aleksandar Markovic <aleksandar.markovic@rt-rk.com> writes:

> From: Leon Alrae <leon.alrae@imgtec.com>
>
> Do only virtual addresses comaprisons in LL/SC sequence.

Doesn't this make things less correct? I know we currently use virtual
addresses for the ARM code but in theory you could map two virtual pages
to the same physical page and then violate the LL/SC behaviour.

Of course I can't imagine why you might do that.

>
> Until this patch, physical addresses had been compared in LL/SC
> sequence. Unfortunately, that meant that, on each SC, the address
> translation had to be done, which is a quite complex operation.
> Getting rid of that allows throwing away SC helpers and having
> common SC implementations in user and system mode, avoiding two
> separate implementations selected by #ifdef CONFIG_USER_ONLY.
>
> Given that LL/SC emulation was already very simplified (as only the
> address and value were compared), using virtual addresses instead of
> physical does not seem to be a violation. Correct guest software
> should not rely on LL/SC if they accesses the same physical address
> via different virtual addresses or if page mapping gets changed
> between LL/SC due to manipulating TLB entries. MIPS Instruction Set
> Manual clearly says that an RMW sequence must use the same address
> in the LL and SC (virtual address, physical address, cacheability
> and coherency attributes must be identical). Otherwise, the result of
> the SC is not predictable. This patch takes advantage of this fact
> and removes the virtual->physical address translation from SC helper.
>
> lladdr served as Coprocessor 0 LLAddr register which captures physical
> address of the most recent LL instruction, and also lladdr was used
> for comparison with following SC physical address. This patch changes
> the meaning of lladdr - now it will only keep the virtual address of
> the most recent LL. Additionally we introduce CP0_LLAddr which is the
> actual Coperocessor 0 LLAddr register that guest can access.
>
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>  target/mips/cpu.h       |  3 ++-
>  target/mips/machine.c   |  7 ++++---
>  target/mips/op_helper.c | 29 +++++++++++++++++------------
>  target/mips/translate.c |  4 ++--
>  4 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 7f8ba5f..57ca861 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -480,10 +480,11 @@ struct CPUMIPSState {
>  #define CP0C5_NFExists   0
>      int32_t CP0_Config6;
>      int32_t CP0_Config7;
> +    uint64_t CP0_LLAddr;
>      uint64_t CP0_MAAR[MIPS_MAAR_MAX];
>      int32_t CP0_MAARI;
>      /* XXX: Maybe make LLAddr per-TC? */
> -    uint64_t lladdr;
> +    target_ulong lladdr; /* LL virtual address compared against SC */
>      target_ulong llval;
>      target_ulong llnewval;
>      target_ulong llreg;
> diff --git a/target/mips/machine.c b/target/mips/machine.c
> index 20100d5..155170c 100644
> --- a/target/mips/machine.c
> +++ b/target/mips/machine.c
> @@ -212,8 +212,8 @@ const VMStateDescription vmstate_tlb = {
>
>  const VMStateDescription vmstate_mips_cpu = {
>      .name = "cpu",
> -    .version_id = 10,
> -    .minimum_version_id = 10,
> +    .version_id = 11,
> +    .minimum_version_id = 11,
>      .post_load = cpu_post_load,
>      .fields = (VMStateField[]) {
>          /* Active TC */
> @@ -283,9 +283,10 @@ const VMStateDescription vmstate_mips_cpu = {
>          VMSTATE_INT32(env.CP0_Config3, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config6, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config7, MIPSCPU),
> +        VMSTATE_UINT64(env.CP0_LLAddr, MIPSCPU),
>          VMSTATE_UINT64_ARRAY(env.CP0_MAAR, MIPSCPU, MIPS_MAAR_MAX),
>          VMSTATE_INT32(env.CP0_MAARI, MIPSCPU),
> -        VMSTATE_UINT64(env.lladdr, MIPSCPU),
> +        VMSTATE_UINTTL(env.lladdr, MIPSCPU),
>          VMSTATE_UINTTL_ARRAY(env.CP0_WatchLo, MIPSCPU, 8),
>          VMSTATE_INT32_ARRAY(env.CP0_WatchHi, MIPSCPU, 8),
>          VMSTATE_UINTTL(env.CP0_XContext, MIPSCPU),
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index e537a8b..283669c 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -255,15 +255,15 @@ static inline hwaddr do_translate_address(CPUMIPSState *env,
>                                                        target_ulong address,
>                                                        int rw, uintptr_t retaddr)
>  {
> -    hwaddr lladdr;
> +    hwaddr paddr;
>      CPUState *cs = CPU(mips_env_get_cpu(env));
>
> -    lladdr = cpu_mips_translate_address(env, address, rw);
> +    paddr = cpu_mips_translate_address(env, address, rw);
>
> -    if (lladdr == -1LL) {
> +    if (paddr == -1LL) {
>          cpu_loop_exit_restore(cs, retaddr);
>      } else {
> -        return lladdr;
> +        return paddr;
>      }
>  }
>
> @@ -274,7 +274,8 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx)  \
>          env->CP0_BadVAddr = arg;                                              \
>          do_raise_exception(env, EXCP_AdEL, GETPC());                          \
>      }                                                                         \
> -    env->lladdr = do_translate_address(env, arg, 0, GETPC());                 \
> +    env->CP0_LLAddr = do_translate_address(env, arg, 0, GETPC());             \
> +    env->lladdr = arg;                                                        \
>      env->llval = do_##insn(env, arg, mem_idx, GETPC());                       \
>      return env->llval;                                                        \
>  }
> @@ -294,7 +295,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,              \
>          env->CP0_BadVAddr = arg2;                                             \
>          do_raise_exception(env, EXCP_AdES, GETPC());                          \
>      }                                                                         \
> -    if (do_translate_address(env, arg2, 1, GETPC()) == env->lladdr) {         \
> +    if (arg2 == env->lladdr) {                                                \
>          tmp = do_##ld_insn(env, arg2, mem_idx, GETPC());                      \
>          if (tmp == env->llval) {                                              \
>              do_##st_insn(env, arg2, arg1, mem_idx, GETPC());                  \
> @@ -873,7 +874,7 @@ target_ulong helper_mftc0_status(CPUMIPSState *env)
>
>  target_ulong helper_mfc0_lladdr(CPUMIPSState *env)
>  {
> -    return (int32_t)(env->lladdr >> env->CP0_LLAddr_shift);
> +    return (int32_t)(env->CP0_LLAddr >> env->CP0_LLAddr_shift);
>  }
>
>  target_ulong helper_mfc0_maar(CPUMIPSState *env)
> @@ -949,7 +950,7 @@ target_ulong helper_dmfc0_tcschefback(CPUMIPSState *env)
>
>  target_ulong helper_dmfc0_lladdr(CPUMIPSState *env)
>  {
> -    return env->lladdr >> env->CP0_LLAddr_shift;
> +    return env->CP0_LLAddr >> env->CP0_LLAddr_shift;
>  }
>
>  target_ulong helper_dmfc0_maar(CPUMIPSState *env)
> @@ -1177,7 +1178,8 @@ void helper_mtc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
>  {
>      env->active_tc.PC = arg1;
>      env->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
> -    env->lladdr = 0ULL;
> +    env->CP0_LLAddr = 0;
> +    env->lladdr = 0;
>      /* MIPS16 not implemented. */
>  }
>
> @@ -1189,12 +1191,14 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
>      if (other_tc == other->current_tc) {
>          other->active_tc.PC = arg1;
>          other->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
> -        other->lladdr = 0ULL;
> +        other->CP0_LLAddr = 0;
> +        other->lladdr = 0;
>          /* MIPS16 not implemented. */
>      } else {
>          other->tcs[other_tc].PC = arg1;
>          other->tcs[other_tc].CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
> -        other->lladdr = 0ULL;
> +        other->CP0_LLAddr = 0;
> +        other->lladdr = 0;
>          /* MIPS16 not implemented. */
>      }
>  }
> @@ -1620,7 +1624,7 @@ void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1)
>  {
>      target_long mask = env->CP0_LLAddr_rw_bitmask;
>      arg1 = arg1 << env->CP0_LLAddr_shift;
> -    env->lladdr = (env->lladdr & ~mask) | (arg1 & mask);
> +    env->CP0_LLAddr = (env->CP0_LLAddr & ~mask) | (arg1 & mask);
>  }
>
>  #define MTC0_MAAR_MASK(env) \
> @@ -2318,6 +2322,7 @@ static inline void exception_return(CPUMIPSState *env)
>  void helper_eret(CPUMIPSState *env)
>  {
>      exception_return(env);
> +    env->CP0_LLAddr = 1;
>      env->lladdr = 1;
>  }
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index d05ee67..c9104a7 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -4913,7 +4913,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>      case 17:
>          switch (sel) {
>          case 0:
> -            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, lladdr),
> +            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, CP0_LLAddr),
>                               ctx->CP0_LLAddr_shift);
>              rn = "LLAddr";
>              break;
> @@ -20440,7 +20440,7 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>                  env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
>      cpu_fprintf(f, "    Config0 0x%08x Config1 0x%08x LLAddr 0x%016"
>                  PRIx64 "\n",
> -                env->CP0_Config0, env->CP0_Config1, env->lladdr);
> +                env->CP0_Config0, env->CP0_Config1, env->CP0_LLAddr);
>      cpu_fprintf(f, "    Config2 0x%08x Config3 0x%08x\n",
>                  env->CP0_Config2, env->CP0_Config3);
>      cpu_fprintf(f, "    Config4 0x%08x Config5 0x%08x\n",


--
Alex Bennée

Re: [Qemu-devel] [PATCH 1/7] target/mips: compare virtual addresses in LL/SC sequence

[Miodrag Dinic]

Hi Alex,

thank you for your comments, and sorry for such a late response, Aleksandar is currently on sick leave, but I will try to address your concern.
Actually, answer to your question is already in the commit message of this patch, I'm copying it here for convenience:

    Given our LL/SC emulation is already very simplified (as we only compare
    the address and value), using virtual addresses instead of physical does
    not seem to be a gross violation. Correct guest software should not rely
    on LL/SC if they accesses the same physical address via different virtual
    addresses or if page mapping gets changed between LL/SC due to manipulating
    tlb entries. MIPS Instruction Set Manual clearly says that an RMW sequence
    must use the same address in the LL and SC (virtual address, physical
    address, cacheability and coherency attributes must be identical). Otherwise
    the result of the SC is not predictable. This patch takes advantage of this
    fact and removes the virtual -> physical address translation from SC helper.

Details about this can be found in the official documentation about SC (page 450) and SCD (page 454) : 
https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00087-2B-MIPS64BIS-AFP-6.06.pdf

Kind regards,
Miodrag
________________________________________
From: Alex Bennée [alex.bennee@linaro.org]
Sent: Friday, January 19, 2018 5:29 PM
To: Aleksandar Markovic
Cc: qemu-devel@nongnu.org; Aurelien Jarno; Fam Zheng; Gerd Hoffmann; Laurent Vivier; Paolo Bonzini; Peter Maydell; Philippe Mathieu-Daudé; Richard Henderson; Riku Voipio; Yongbok Kim; Aleksandar Markovic; Goran Ferenc; Miodrag Dinic; Petar Jovanovic
Subject: Re: [PATCH 1/7] target/mips: compare virtual addresses in LL/SC sequence

Aleksandar Markovic <aleksandar.markovic@rt-rk.com> writes:

> From: Leon Alrae <leon.alrae@imgtec.com>
>
> Do only virtual addresses comaprisons in LL/SC sequence.

Doesn't this make things less correct? I know we currently use virtual
addresses for the ARM code but in theory you could map two virtual pages
to the same physical page and then violate the LL/SC behaviour.

Of course I can't imagine why you might do that.

>
> Until this patch, physical addresses had been compared in LL/SC
> sequence. Unfortunately, that meant that, on each SC, the address
> translation had to be done, which is a quite complex operation.
> Getting rid of that allows throwing away SC helpers and having
> common SC implementations in user and system mode, avoiding two
> separate implementations selected by #ifdef CONFIG_USER_ONLY.
>
> Given that LL/SC emulation was already very simplified (as only the
> address and value were compared), using virtual addresses instead of
> physical does not seem to be a violation. Correct guest software
> should not rely on LL/SC if they accesses the same physical address
> via different virtual addresses or if page mapping gets changed
> between LL/SC due to manipulating TLB entries. MIPS Instruction Set
> Manual clearly says that an RMW sequence must use the same address
> in the LL and SC (virtual address, physical address, cacheability
> and coherency attributes must be identical). Otherwise, the result of
> the SC is not predictable. This patch takes advantage of this fact
> and removes the virtual->physical address translation from SC helper.
>
> lladdr served as Coprocessor 0 LLAddr register which captures physical
> address of the most recent LL instruction, and also lladdr was used
> for comparison with following SC physical address. This patch changes
> the meaning of lladdr - now it will only keep the virtual address of
> the most recent LL. Additionally we introduce CP0_LLAddr which is the
> actual Coperocessor 0 LLAddr register that guest can access.
>
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>  target/mips/cpu.h       |  3 ++-
>  target/mips/machine.c   |  7 ++++---
>  target/mips/op_helper.c | 29 +++++++++++++++++------------
>  target/mips/translate.c |  4 ++--
>  4 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 7f8ba5f..57ca861 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -480,10 +480,11 @@ struct CPUMIPSState {
>  #define CP0C5_NFExists   0
>      int32_t CP0_Config6;
>      int32_t CP0_Config7;
> +    uint64_t CP0_LLAddr;
>      uint64_t CP0_MAAR[MIPS_MAAR_MAX];
>      int32_t CP0_MAARI;
>      /* XXX: Maybe make LLAddr per-TC? */
> -    uint64_t lladdr;
> +    target_ulong lladdr; /* LL virtual address compared against SC */
>      target_ulong llval;
>      target_ulong llnewval;
>      target_ulong llreg;
> diff --git a/target/mips/machine.c b/target/mips/machine.c
> index 20100d5..155170c 100644
> --- a/target/mips/machine.c
> +++ b/target/mips/machine.c
> @@ -212,8 +212,8 @@ const VMStateDescription vmstate_tlb = {
>
>  const VMStateDescription vmstate_mips_cpu = {
>      .name = "cpu",
> -    .version_id = 10,
> -    .minimum_version_id = 10,
> +    .version_id = 11,
> +    .minimum_version_id = 11,
>      .post_load = cpu_post_load,
>      .fields = (VMStateField[]) {
>          /* Active TC */
> @@ -283,9 +283,10 @@ const VMStateDescription vmstate_mips_cpu = {
>          VMSTATE_INT32(env.CP0_Config3, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config6, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config7, MIPSCPU),
> +        VMSTATE_UINT64(env.CP0_LLAddr, MIPSCPU),
>          VMSTATE_UINT64_ARRAY(env.CP0_MAAR, MIPSCPU, MIPS_MAAR_MAX),
>          VMSTATE_INT32(env.CP0_MAARI, MIPSCPU),
> -        VMSTATE_UINT64(env.lladdr, MIPSCPU),
> +        VMSTATE_UINTTL(env.lladdr, MIPSCPU),
>          VMSTATE_UINTTL_ARRAY(env.CP0_WatchLo, MIPSCPU, 8),
>          VMSTATE_INT32_ARRAY(env.CP0_WatchHi, MIPSCPU, 8),
>          VMSTATE_UINTTL(env.CP0_XContext, MIPSCPU),
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index e537a8b..283669c 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -255,15 +255,15 @@ static inline hwaddr do_translate_address(CPUMIPSState *env,
>                                                        target_ulong address,
>                                                        int rw, uintptr_t retaddr)
>  {
> -    hwaddr lladdr;
> +    hwaddr paddr;
>      CPUState *cs = CPU(mips_env_get_cpu(env));
>
> -    lladdr = cpu_mips_translate_address(env, address, rw);
> +    paddr = cpu_mips_translate_address(env, address, rw);
>
> -    if (lladdr == -1LL) {
> +    if (paddr == -1LL) {
>          cpu_loop_exit_restore(cs, retaddr);
>      } else {
> -        return lladdr;
> +        return paddr;
>      }
>  }
>
> @@ -274,7 +274,8 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx)  \
>          env->CP0_BadVAddr = arg;                                              \
>          do_raise_exception(env, EXCP_AdEL, GETPC());                          \
>      }                                                                         \
> -    env->lladdr = do_translate_address(env, arg, 0, GETPC());                 \
> +    env->CP0_LLAddr = do_translate_address(env, arg, 0, GETPC());             \
> +    env->lladdr = arg;                                                        \
>      env->llval = do_##insn(env, arg, mem_idx, GETPC());                       \
>      return env->llval;                                                        \
>  }
> @@ -294,7 +295,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,              \
>          env->CP0_BadVAddr = arg2;                                             \
>          do_raise_exception(env, EXCP_AdES, GETPC());                          \
>      }                                                                         \
> -    if (do_translate_address(env, arg2, 1, GETPC()) == env->lladdr) {         \
> +    if (arg2 == env->lladdr) {                                                \
>          tmp = do_##ld_insn(env, arg2, mem_idx, GETPC());                      \
>          if (tmp == env->llval) {                                              \
>              do_##st_insn(env, arg2, arg1, mem_idx, GETPC());                  \
> @@ -873,7 +874,7 @@ target_ulong helper_mftc0_status(CPUMIPSState *env)
>
>  target_ulong helper_mfc0_lladdr(CPUMIPSState *env)
>  {
> -    return (int32_t)(env->lladdr >> env->CP0_LLAddr_shift);
> +    return (int32_t)(env->CP0_LLAddr >> env->CP0_LLAddr_shift);
>  }
>
>  target_ulong helper_mfc0_maar(CPUMIPSState *env)
> @@ -949,7 +950,7 @@ target_ulong helper_dmfc0_tcschefback(CPUMIPSState *env)
>
>  target_ulong helper_dmfc0_lladdr(CPUMIPSState *env)
>  {
> -    return env->lladdr >> env->CP0_LLAddr_shift;
> +    return env->CP0_LLAddr >> env->CP0_LLAddr_shift;
>  }
>
>  target_ulong helper_dmfc0_maar(CPUMIPSState *env)
> @@ -1177,7 +1178,8 @@ void helper_mtc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
>  {
>      env->active_tc.PC = arg1;
>      env->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
> -    env->lladdr = 0ULL;
> +    env->CP0_LLAddr = 0;
> +    env->lladdr = 0;
>      /* MIPS16 not implemented. */
>  }
>
> @@ -1189,12 +1191,14 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
>      if (other_tc == other->current_tc) {
>          other->active_tc.PC = arg1;
>          other->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
> -        other->lladdr = 0ULL;
> +        other->CP0_LLAddr = 0;
> +        other->lladdr = 0;
>          /* MIPS16 not implemented. */
>      } else {
>          other->tcs[other_tc].PC = arg1;
>          other->tcs[other_tc].CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
> -        other->lladdr = 0ULL;
> +        other->CP0_LLAddr = 0;
> +        other->lladdr = 0;
>          /* MIPS16 not implemented. */
>      }
>  }
> @@ -1620,7 +1624,7 @@ void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1)
>  {
>      target_long mask = env->CP0_LLAddr_rw_bitmask;
>      arg1 = arg1 << env->CP0_LLAddr_shift;
> -    env->lladdr = (env->lladdr & ~mask) | (arg1 & mask);
> +    env->CP0_LLAddr = (env->CP0_LLAddr & ~mask) | (arg1 & mask);
>  }
>
>  #define MTC0_MAAR_MASK(env) \
> @@ -2318,6 +2322,7 @@ static inline void exception_return(CPUMIPSState *env)
>  void helper_eret(CPUMIPSState *env)
>  {
>      exception_return(env);
> +    env->CP0_LLAddr = 1;
>      env->lladdr = 1;
>  }
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index d05ee67..c9104a7 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -4913,7 +4913,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>      case 17:
>          switch (sel) {
>          case 0:
> -            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, lladdr),
> +            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, CP0_LLAddr),
>                               ctx->CP0_LLAddr_shift);
>              rn = "LLAddr";
>              break;
> @@ -20440,7 +20440,7 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>                  env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
>      cpu_fprintf(f, "    Config0 0x%08x Config1 0x%08x LLAddr 0x%016"
>                  PRIx64 "\n",
> -                env->CP0_Config0, env->CP0_Config1, env->lladdr);
> +                env->CP0_Config0, env->CP0_Config1, env->CP0_LLAddr);
>      cpu_fprintf(f, "    Config2 0x%08x Config3 0x%08x\n",
>                  env->CP0_Config2, env->CP0_Config3);
>      cpu_fprintf(f, "    Config4 0x%08x Config5 0x%08x\n",


--
Alex Bennée

[Qemu-devel] [PATCH 2/7] target/mips: reimplement SC instruction and use cmpxchg

[Aleksandar Markovic]

From: Leon Alrae <lean.alrae@imgtec.com>

Completely rewrite conditional stores handling. Use cmpxchg.

This eliminates need for separate implementations for user and system
emulation.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/main.c       |  58 ------------------------
 target/mips/cpu.h       |   4 --
 target/mips/helper.c    |   6 +--
 target/mips/helper.h    |   2 -
 target/mips/op_helper.c |  25 -----------
 target/mips/translate.c | 114 ++++++++++++++++--------------------------------
 6 files changed, 39 insertions(+), 170 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 450eb3c..91977f8 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2155,55 +2155,6 @@ static const uint8_t mips_syscall_args[] = {
 #  undef MIPS_SYS
 # endif /* O32 */
 
-static int do_store_exclusive(CPUMIPSState *env)
-{
-    target_ulong addr;
-    target_ulong page_addr;
-    target_ulong val;
-    int flags;
-    int segv = 0;
-    int reg;
-    int d;
-
-    addr = env->lladdr;
-    page_addr = addr & TARGET_PAGE_MASK;
-    start_exclusive();
-    mmap_lock();
-    flags = page_get_flags(page_addr);
-    if ((flags & PAGE_READ) == 0) {
-        segv = 1;
-    } else {
-        reg = env->llreg & 0x1f;
-        d = (env->llreg & 0x20) != 0;
-        if (d) {
-            segv = get_user_s64(val, addr);
-        } else {
-            segv = get_user_s32(val, addr);
-        }
-        if (!segv) {
-            if (val != env->llval) {
-                env->active_tc.gpr[reg] = 0;
-            } else {
-                if (d) {
-                    segv = put_user_u64(env->llnewval, addr);
-                } else {
-                    segv = put_user_u32(env->llnewval, addr);
-                }
-                if (!segv) {
-                    env->active_tc.gpr[reg] = 1;
-                }
-            }
-        }
-    }
-    env->lladdr = -1;
-    if (!segv) {
-        env->active_tc.PC += 4;
-    }
-    mmap_unlock();
-    end_exclusive();
-    return segv;
-}
-
 /* Break codes */
 enum {
     BRK_OVERFLOW = 6,
@@ -2353,15 +2304,6 @@ done_syscall:
                   }
             }
             break;
-        case EXCP_SC:
-            if (do_store_exclusive(env)) {
-                info.si_signo = TARGET_SIGSEGV;
-                info.si_errno = 0;
-                info.si_code = TARGET_SEGV_MAPERR;
-                info._sifields._sigfault._addr = env->active_tc.PC;
-                queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
-            }
-            break;
         case EXCP_DSPDIS:
             info.si_signo = TARGET_SIGILL;
             info.si_errno = 0;
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 57ca861..3fa85b0 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -486,8 +486,6 @@ struct CPUMIPSState {
     /* XXX: Maybe make LLAddr per-TC? */
     target_ulong lladdr; /* LL virtual address compared against SC */
     target_ulong llval;
-    target_ulong llnewval;
-    target_ulong llreg;
     uint64_t CP0_LLAddr_rw_bitmask;
     int CP0_LLAddr_shift;
     target_ulong CP0_WatchLo[8];
@@ -727,8 +725,6 @@ enum {
 
     EXCP_LAST = EXCP_TLBRI,
 };
-/* Dummy exception for conditional stores.  */
-#define EXCP_SC 0x100
 
 /*
  * This is an internally generated WAKE request line.
diff --git a/target/mips/helper.c b/target/mips/helper.c
index ea07626..b34f01a 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -1084,10 +1084,8 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
 {
     CPUState *cs = CPU(mips_env_get_cpu(env));
 
-    if (exception < EXCP_SC) {
-        qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
-                      __func__, exception, error_code);
-    }
+    qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
+                  __func__, exception, error_code);
     cs->exception_index = exception;
     env->error_code = error_code;
 
diff --git a/target/mips/helper.h b/target/mips/helper.h
index 5f49234..04c36e9 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -13,10 +13,8 @@ DEF_HELPER_4(swr, void, env, tl, tl, int)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(ll, tl, env, tl, int)
-DEF_HELPER_4(sc, tl, env, tl, tl, int)
 #ifdef TARGET_MIPS64
 DEF_HELPER_3(lld, tl, env, tl, int)
-DEF_HELPER_4(scd, tl, env, tl, tl, int)
 #endif
 #endif
 
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 283669c..a429987 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -284,31 +284,6 @@ HELPER_LD_ATOMIC(ll, lw, 0x3)
 HELPER_LD_ATOMIC(lld, ld, 0x7)
 #endif
 #undef HELPER_LD_ATOMIC
-
-#define HELPER_ST_ATOMIC(name, ld_insn, st_insn, almask)                      \
-target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,              \
-                           target_ulong arg2, int mem_idx)                    \
-{                                                                             \
-    target_long tmp;                                                          \
-                                                                              \
-    if (arg2 & almask) {                                                      \
-        env->CP0_BadVAddr = arg2;                                             \
-        do_raise_exception(env, EXCP_AdES, GETPC());                          \
-    }                                                                         \
-    if (arg2 == env->lladdr) {                                                \
-        tmp = do_##ld_insn(env, arg2, mem_idx, GETPC());                      \
-        if (tmp == env->llval) {                                              \
-            do_##st_insn(env, arg2, arg1, mem_idx, GETPC());                  \
-            return 1;                                                         \
-        }                                                                     \
-    }                                                                         \
-    return 0;                                                                 \
-}
-HELPER_ST_ATOMIC(sc, lw, sw, 0x3)
-#ifdef TARGET_MIPS64
-HELPER_ST_ATOMIC(scd, ld, sd, 0x7)
-#endif
-#undef HELPER_ST_ATOMIC
 #endif
 
 #ifdef TARGET_WORDS_BIGENDIAN
diff --git a/target/mips/translate.c b/target/mips/translate.c
index c9104a7..f851d41 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1383,6 +1383,7 @@ static TCGv_i32 hflags;
 static TCGv_i32 fpu_fcr0, fpu_fcr31;
 static TCGv_i64 fpu_f64[32];
 static TCGv_i64 msa_wr_d[64];
+static TCGv cpu_lladdr, cpu_llval;
 
 #include "exec/gen-icount.h"
 
@@ -2073,48 +2074,6 @@ OP_LD_ATOMIC(lld,ld64);
 #endif
 #undef OP_LD_ATOMIC
 
-#ifdef CONFIG_USER_ONLY
-#define OP_ST_ATOMIC(insn,fname,ldname,almask)                               \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, int mem_idx,   \
-                                DisasContext *ctx)                           \
-{                                                                            \
-    TCGv t0 = tcg_temp_new();                                                \
-    TCGLabel *l1 = gen_new_label();                                          \
-    TCGLabel *l2 = gen_new_label();                                          \
-                                                                             \
-    tcg_gen_andi_tl(t0, arg2, almask);                                       \
-    tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, l1);                              \
-    tcg_gen_st_tl(arg2, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr));          \
-    generate_exception(ctx, EXCP_AdES);                                      \
-    gen_set_label(l1);                                                       \
-    tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUMIPSState, lladdr));                  \
-    tcg_gen_brcond_tl(TCG_COND_NE, arg2, t0, l2);                            \
-    tcg_gen_movi_tl(t0, rt | ((almask << 3) & 0x20));                        \
-    tcg_gen_st_tl(t0, cpu_env, offsetof(CPUMIPSState, llreg));                   \
-    tcg_gen_st_tl(arg1, cpu_env, offsetof(CPUMIPSState, llnewval));              \
-    generate_exception_end(ctx, EXCP_SC);                                    \
-    gen_set_label(l2);                                                       \
-    tcg_gen_movi_tl(t0, 0);                                                  \
-    gen_store_gpr(t0, rt);                                                   \
-    tcg_temp_free(t0);                                                       \
-}
-#else
-#define OP_ST_ATOMIC(insn,fname,ldname,almask)                               \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, int mem_idx,   \
-                                DisasContext *ctx)                           \
-{                                                                            \
-    TCGv t0 = tcg_temp_new();                                                \
-    gen_helper_1e2i(insn, t0, arg1, arg2, mem_idx);                          \
-    gen_store_gpr(t0, rt);                                                   \
-    tcg_temp_free(t0);                                                       \
-}
-#endif
-OP_ST_ATOMIC(sc,st32,ld32s,0x3);
-#if defined(TARGET_MIPS64)
-OP_ST_ATOMIC(scd,st64,ld64,0x7);
-#endif
-#undef OP_ST_ATOMIC
-
 static void gen_base_offset_addr (DisasContext *ctx, TCGv addr,
                                   int base, int16_t offset)
 {
@@ -2401,37 +2360,34 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
 
 
 /* Store conditional */
-static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
-                         int base, int16_t offset)
+static void gen_st_cond(DisasContext *ctx, int rt, int base, int offset,
+                        TCGMemOp tcg_mo)
 {
-    TCGv t0, t1;
-    int mem_idx = ctx->mem_idx;
+    TCGv addr, t0, val;
+    TCGLabel *l1 = gen_new_label();
+    TCGLabel *done = gen_new_label();
+
+     t0 = tcg_temp_new();
+    addr = tcg_temp_new();
+    /* compare the address against that of the preceeding LL */
+    gen_base_offset_addr(ctx, addr, base, offset);
+    tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l1);
+    tcg_temp_free(addr);
+    tcg_gen_movi_tl(t0, 0);
+    gen_store_gpr(t0, rt);
+    tcg_gen_br(done);
 
-#ifdef CONFIG_USER_ONLY
-    t0 = tcg_temp_local_new();
-    t1 = tcg_temp_local_new();
-#else
-    t0 = tcg_temp_new();
-    t1 = tcg_temp_new();
-#endif
-    gen_base_offset_addr(ctx, t0, base, offset);
-    gen_load_gpr(t1, rt);
-    switch (opc) {
-#if defined(TARGET_MIPS64)
-    case OPC_SCD:
-    case R6_OPC_SCD:
-        op_st_scd(t1, t0, rt, mem_idx, ctx);
-        break;
-#endif
-    case OPC_SCE:
-        mem_idx = MIPS_HFLAG_UM;
-        /* fall through */
-    case OPC_SC:
-    case R6_OPC_SC:
-        op_st_sc(t1, t0, rt, mem_idx, ctx);
-        break;
-    }
-    tcg_temp_free(t1);
+    gen_set_label(l1);
+    /* generate cmpxchg */
+    val = tcg_temp_new();
+    gen_load_gpr(val, rt);
+    tcg_gen_atomic_cmpxchg_tl(t0, cpu_lladdr, cpu_llval, val,
+                              ctx->mem_idx, tcg_mo);
+    tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_llval);
+    gen_store_gpr(t0, rt);
+    tcg_temp_free(val);
+
+    gen_set_label(done);
     tcg_temp_free(t0);
 }
 
@@ -14889,13 +14845,13 @@ static void decode_micromips32_opc(CPUMIPSState *env, DisasContext *ctx)
             gen_st(ctx, mips32_op, rt, rs, offset);
             break;
         case SC:
-            gen_st_cond(ctx, OPC_SC, rt, rs, offset);
+            gen_st_cond(ctx, rt, rs, offset, MO_TESL);
             break;
 #if defined(TARGET_MIPS64)
         case SCD:
             check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
-            gen_st_cond(ctx, OPC_SCD, rt, rs, offset);
+            gen_st_cond(ctx, rt, rs, offset, MO_TEQ);
             break;
 #endif
         case LD_EVA:
@@ -17695,7 +17651,7 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
     case R6_OPC_SC:
-        gen_st_cond(ctx, op1, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, MO_TESL);
         break;
     case R6_OPC_LL:
         gen_ld(ctx, op1, rt, rs, imm);
@@ -17719,7 +17675,7 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx)
         break;
 #if defined(TARGET_MIPS64)
     case R6_OPC_SCD:
-        gen_st_cond(ctx, op1, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, MO_TEQ);
         break;
     case R6_OPC_LLD:
         gen_ld(ctx, op1, rt, rs, imm);
@@ -19844,7 +19800,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
     case OPC_SC:
         check_insn(ctx, ISA_MIPS2);
          check_insn_opc_removed(ctx, ISA_MIPS32R6);
-         gen_st_cond(ctx, op, rt, rs, imm);
+         gen_st_cond(ctx, rt, rs, imm, MO_TESL);
          break;
     case OPC_CACHE:
         check_insn_opc_removed(ctx, ISA_MIPS32R6);
@@ -20130,7 +20086,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         check_insn_opc_removed(ctx, ISA_MIPS32R6);
         check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
-        gen_st_cond(ctx, op, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, MO_TEQ);
         break;
     case OPC_BNVC: /* OPC_BNEZALC, OPC_BNEC, OPC_DADDI */
         if (ctx->insn_flags & ISA_MIPS32R6) {
@@ -20497,6 +20453,10 @@ void mips_tcg_init(void)
     fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
+    cpu_lladdr = tcg_global_mem_new(cpu_env, offsetof(CPUMIPSState, lladdr),
+                                    "lladdr");
+    cpu_llval = tcg_global_mem_new(cpu_env, offsetof(CPUMIPSState, llval),
+                                   "llval");
 }
 
 #include "translate_init.c"
-- 
2.7.4

[Qemu-devel] [PATCH 3/7] Revert "target/mips: hold BQL for timer interrupts"

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@mips.com>

This reverts commit d394698d73836d1c50545bdb32dc58d09708fcfb.

Ther revert is needed in order not to cause overlap with subsequent
patches related to handling synchronization of interrupt code paths.

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 target/mips/op_helper.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index a429987..3830ee8 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -17,7 +17,6 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
-#include "qemu/main-loop.h"
 #include "cpu.h"
 #include "internal.h"
 #include "qemu/host-utils.h"
@@ -809,11 +808,7 @@ target_ulong helper_mftc0_tcschefback(CPUMIPSState *env)
 
 target_ulong helper_mfc0_count(CPUMIPSState *env)
 {
-    int32_t count;
-    qemu_mutex_lock_iothread();
-    count = (int32_t) cpu_mips_get_count(env);
-    qemu_mutex_unlock_iothread();
-    return count;
+    return (int32_t)cpu_mips_get_count(env);
 }
 
 target_ulong helper_mftc0_entryhi(CPUMIPSState *env)
@@ -1388,9 +1383,7 @@ void helper_mtc0_hwrena(CPUMIPSState *env, target_ulong arg1)
 
 void helper_mtc0_count(CPUMIPSState *env, target_ulong arg1)
 {
-    qemu_mutex_lock_iothread();
     cpu_mips_store_count(env, arg1);
-    qemu_mutex_unlock_iothread();
 }
 
 void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
@@ -1439,9 +1432,7 @@ void helper_mttc0_entryhi(CPUMIPSState *env, target_ulong arg1)
 
 void helper_mtc0_compare(CPUMIPSState *env, target_ulong arg1)
 {
-    qemu_mutex_lock_iothread();
     cpu_mips_store_compare(env, arg1);
-    qemu_mutex_unlock_iothread();
 }
 
 void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1)
@@ -1495,9 +1486,7 @@ void helper_mtc0_srsctl(CPUMIPSState *env, target_ulong arg1)
 
 void helper_mtc0_cause(CPUMIPSState *env, target_ulong arg1)
 {
-    qemu_mutex_lock_iothread();
     cpu_mips_store_cause(env, arg1);
-    qemu_mutex_unlock_iothread();
 }
 
 void helper_mttc0_cause(CPUMIPSState *env, target_ulong arg1)
@@ -2339,16 +2328,12 @@ target_ulong helper_rdhwr_synci_step(CPUMIPSState *env)
 
 target_ulong helper_rdhwr_cc(CPUMIPSState *env)
 {
-    int32_t count;
     check_hwrena(env, 2, GETPC());
 #ifdef CONFIG_USER_ONLY
-    count = env->CP0_Count;
+    return env->CP0_Count;
 #else
-    qemu_mutex_lock_iothread();
-    count = (int32_t)cpu_mips_get_count(env);
-    qemu_mutex_unlock_iothread();
+    return (int32_t)cpu_mips_get_count(env);
 #endif
-    return count;
 }
 
 target_ulong helper_rdhwr_ccres(CPUMIPSState *env)
-- 
2.7.4

Re: [Qemu-devel] [PATCH 3/7] Revert "target/mips: hold BQL for timer interrupts"

[Alex Bennée]

Aleksandar Markovic <aleksandar.markovic@rt-rk.com> writes:

> From: Aleksandar Markovic <aleksandar.markovic@mips.com>
>
> This reverts commit d394698d73836d1c50545bdb32dc58d09708fcfb.
>
> Ther revert is needed in order not to cause overlap with subsequent
> patches related to handling synchronization of interrupt code paths.

Hmm I'm fairly sure you should merge this with the follow-up patch to
add BQL locking to the appropriate place. Otherwise you break
bi-section.

>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>  target/mips/op_helper.c | 21 +++------------------
>  1 file changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index a429987..3830ee8 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -17,7 +17,6 @@
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
>  #include "qemu/osdep.h"
> -#include "qemu/main-loop.h"
>  #include "cpu.h"
>  #include "internal.h"
>  #include "qemu/host-utils.h"
> @@ -809,11 +808,7 @@ target_ulong helper_mftc0_tcschefback(CPUMIPSState *env)
>
>  target_ulong helper_mfc0_count(CPUMIPSState *env)
>  {
> -    int32_t count;
> -    qemu_mutex_lock_iothread();
> -    count = (int32_t) cpu_mips_get_count(env);
> -    qemu_mutex_unlock_iothread();
> -    return count;
> +    return (int32_t)cpu_mips_get_count(env);
>  }
>
>  target_ulong helper_mftc0_entryhi(CPUMIPSState *env)
> @@ -1388,9 +1383,7 @@ void helper_mtc0_hwrena(CPUMIPSState *env, target_ulong arg1)
>
>  void helper_mtc0_count(CPUMIPSState *env, target_ulong arg1)
>  {
> -    qemu_mutex_lock_iothread();
>      cpu_mips_store_count(env, arg1);
> -    qemu_mutex_unlock_iothread();
>  }
>
>  void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
> @@ -1439,9 +1432,7 @@ void helper_mttc0_entryhi(CPUMIPSState *env, target_ulong arg1)
>
>  void helper_mtc0_compare(CPUMIPSState *env, target_ulong arg1)
>  {
> -    qemu_mutex_lock_iothread();
>      cpu_mips_store_compare(env, arg1);
> -    qemu_mutex_unlock_iothread();
>  }
>
>  void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1)
> @@ -1495,9 +1486,7 @@ void helper_mtc0_srsctl(CPUMIPSState *env, target_ulong arg1)
>
>  void helper_mtc0_cause(CPUMIPSState *env, target_ulong arg1)
>  {
> -    qemu_mutex_lock_iothread();
>      cpu_mips_store_cause(env, arg1);
> -    qemu_mutex_unlock_iothread();
>  }
>
>  void helper_mttc0_cause(CPUMIPSState *env, target_ulong arg1)
> @@ -2339,16 +2328,12 @@ target_ulong helper_rdhwr_synci_step(CPUMIPSState *env)
>
>  target_ulong helper_rdhwr_cc(CPUMIPSState *env)
>  {
> -    int32_t count;
>      check_hwrena(env, 2, GETPC());
>  #ifdef CONFIG_USER_ONLY
> -    count = env->CP0_Count;
> +    return env->CP0_Count;
>  #else
> -    qemu_mutex_lock_iothread();
> -    count = (int32_t)cpu_mips_get_count(env);
> -    qemu_mutex_unlock_iothread();
> +    return (int32_t)cpu_mips_get_count(env);
>  #endif
> -    return count;
>  }
>
>  target_ulong helper_rdhwr_ccres(CPUMIPSState *env)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 3/7] Revert "target/mips: hold BQL for timer interrupts"

[Aleksandar Markovic]

> From: Alex Bennée [alex.bennee@linaro.org]
> 
> > From: Aleksandar Markovic <aleksandar.markovic@mips.com>
> >
> > This reverts commit d394698d73836d1c50545bdb32dc58d09708fcfb.
> >
> > Ther revert is needed in order not to cause overlap with subsequent
> > patches related to handling synchronization of interrupt code paths.
> 
> Hmm I'm fairly sure you should merge this with the follow-up patch to
> add BQL locking to the appropriate place. Otherwise you break
> bi-section.

Thanks for the review, Alex!

You are right, the bisect would have been broken. I am going to merge this and
subsequent patch in v2.

I am going to address all your other concerns in v2 as well.

Regards,
Aleksandar

[Qemu-devel] [PATCH 4/7] hw/mips_int: hold BQL for all interrupt requests

[Aleksandar Markovic]

From: Miodrag Dinic <miodrag.dinic@mips.com>

Make sure BQL is held for all interrupt requests.

For MTTCG-enabled configurations, handling soft and hard interrupts
between vCPUs must be properly locked. By acquiring BQL, make sure
all paths triggering an IRQ are synchronized.

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 hw/mips/mips_int.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
index 48192d2..5ddeb15 100644
--- a/hw/mips/mips_int.c
+++ b/hw/mips/mips_int.c
@@ -21,6 +21,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 #include "hw/hw.h"
 #include "hw/mips/cpudevs.h"
 #include "cpu.h"
@@ -32,10 +33,17 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
     MIPSCPU *cpu = opaque;
     CPUMIPSState *env = &cpu->env;
     CPUState *cs = CPU(cpu);
+    bool locked = false;
 
     if (irq < 0 || irq > 7)
         return;
 
+    /* Make sure locking works even if BQL is already held by the caller */
+    if (!qemu_mutex_iothread_locked()) {
+        locked = true;
+        qemu_mutex_lock_iothread();
+    }
+
     if (level) {
         env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
 
@@ -56,6 +64,10 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
     } else {
         cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
     }
+
+    if (locked) {
+        qemu_mutex_unlock_iothread();
+    }
 }
 
 void cpu_mips_irq_init_cpu(MIPSCPU *cpu)
-- 
2.7.4

[Qemu-devel] [PATCH 5/7] target/mips: hold BQL in mips_vpe_wake()

[Aleksandar Markovic]

From: Goran Ferenc <goran.ferenc@mips.com>

Hold BQL whenever mips_vpe_wake() is invoked.

Without this patch, MIPS MT with MTTCG enabled triggers an abort in
tcg_handle_interrupt() due to an unlocked access to cpu_interrupt().
This patch makes sure that the BQL is held in this case.

Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 target/mips/op_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 3830ee8..d512d8e 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 #include "cpu.h"
 #include "internal.h"
 #include "qemu/host-utils.h"
@@ -542,7 +543,9 @@ static inline void mips_vpe_wake(MIPSCPU *c)
     /* Don't set ->halted = 0 directly, let it be done via cpu_has_work
        because there might be other conditions that state that c should
        be sleeping.  */
+    qemu_mutex_lock_iothread();
     cpu_interrupt(CPU(c), CPU_INTERRUPT_WAKE);
+    qemu_mutex_unlock_iothread();
 }
 
 static inline void mips_vpe_sleep(MIPSCPU *cpu)
-- 
2.7.4

[Qemu-devel] [PATCH 6/7] hw/mips_cpc: kick a VP when putting it into Run state

[Aleksandar Markovic]

From: Miodrag Dinic <miodrag.dinic@mips.com>

While testing mttcg VP0 could get gets stuck in a loop waiting
for other VPs to come up (which never actually happens). To fix this,
kick VPs while they are being powered up by Cluster Power Controller
in a async task which is triggered once the host thread is being
spawned.

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 hw/misc/mips_cpc.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/misc/mips_cpc.c b/hw/misc/mips_cpc.c
index 6d34574..105a109 100644
--- a/hw/misc/mips_cpc.c
+++ b/hw/misc/mips_cpc.c
@@ -30,6 +30,14 @@ static inline uint64_t cpc_vp_run_mask(MIPSCPCState *cpc)
     return (1ULL << cpc->num_vp) - 1;
 }
 
+static void mips_cpu_reset_async_work(CPUState *cs, run_on_cpu_data data)
+{
+    MIPSCPCState *cpc = (MIPSCPCState *) data.host_ptr;
+
+    cpu_reset(cs);
+    cpc->vp_running |= 1ULL << cs->cpu_index;
+}
+
 static void cpc_run_vp(MIPSCPCState *cpc, uint64_t vp_run)
 {
     CPUState *cs = first_cpu;
@@ -37,8 +45,13 @@ static void cpc_run_vp(MIPSCPCState *cpc, uint64_t vp_run)
     CPU_FOREACH(cs) {
         uint64_t i = 1ULL << cs->cpu_index;
         if (i & vp_run & ~cpc->vp_running) {
-            cpu_reset(cs);
-            cpc->vp_running |= i;
+            /*
+             * To avoid racing with a CPU we are just kicking off.
+             * We do the final bit of preparation for the work in
+             * the target CPUs context.
+             */
+            async_run_on_cpu(cs, mips_cpu_reset_async_work,
+                             RUN_ON_CPU_HOST_PTR(cpc));
         }
     }
 }
-- 
2.7.4

Re: [Qemu-devel] [PATCH 6/7] hw/mips_cpc: kick a VP when putting it into Run state

[Alex Bennée]

Aleksandar Markovic <aleksandar.markovic@rt-rk.com> writes:

> From: Miodrag Dinic <miodrag.dinic@mips.com>
>
> While testing mttcg VP0 could get gets stuck in a loop waiting
> for other VPs to come up (which never actually happens). To fix this,
> kick VPs while they are being powered up by Cluster Power Controller
> in a async task which is triggered once the host thread is being
> spawned.
>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>  hw/misc/mips_cpc.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/mips_cpc.c b/hw/misc/mips_cpc.c
> index 6d34574..105a109 100644
> --- a/hw/misc/mips_cpc.c
> +++ b/hw/misc/mips_cpc.c
> @@ -30,6 +30,14 @@ static inline uint64_t cpc_vp_run_mask(MIPSCPCState *cpc)
>      return (1ULL << cpc->num_vp) - 1;
>  }
>
> +static void mips_cpu_reset_async_work(CPUState *cs, run_on_cpu_data data)
> +{
> +    MIPSCPCState *cpc = (MIPSCPCState *) data.host_ptr;

Is MIPSCPCState part of the CPUState or a shared structure? If it is a
shared structure you may need to use safe work or have some form of
locking to prevent races.

> +
> +    cpu_reset(cs);
> +    cpc->vp_running |= 1ULL << cs->cpu_index;
> +}
> +
>  static void cpc_run_vp(MIPSCPCState *cpc, uint64_t vp_run)
>  {
>      CPUState *cs = first_cpu;
> @@ -37,8 +45,13 @@ static void cpc_run_vp(MIPSCPCState *cpc, uint64_t vp_run)
>      CPU_FOREACH(cs) {
>          uint64_t i = 1ULL << cs->cpu_index;
>          if (i & vp_run & ~cpc->vp_running) {
> -            cpu_reset(cs);
> -            cpc->vp_running |= i;
> +            /*
> +             * To avoid racing with a CPU we are just kicking off.
> +             * We do the final bit of preparation for the work in
> +             * the target CPUs context.
> +             */
> +            async_run_on_cpu(cs, mips_cpu_reset_async_work,
> +                             RUN_ON_CPU_HOST_PTR(cpc));
>          }
>      }
>  }


--
Alex Bennée

[Qemu-devel] [PATCH 7/7] target/mips: introduce MTTCG-enabled builds

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@mips.com>

Introduce MTTCG-enabled QEMU builds for mips32, mipsn32, and mips64.

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 configure         | 3 +++
 target/mips/cpu.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/configure b/configure
index 6d8c996..f516c21 100755
--- a/configure
+++ b/configure
@@ -6534,16 +6534,19 @@ case "$target_name" in
     bflt="yes"
   ;;
   mips|mipsel)
+    mttcg="yes"
     TARGET_ARCH=mips
     echo "TARGET_ABI_MIPSO32=y" >> $config_target_mak
   ;;
   mipsn32|mipsn32el)
+    mttcg="yes"
     TARGET_ARCH=mips64
     TARGET_BASE_ARCH=mips
     echo "TARGET_ABI_MIPSN32=y" >> $config_target_mak
     echo "TARGET_ABI32=y" >> $config_target_mak
   ;;
   mips64|mips64el)
+    mttcg="yes"
     TARGET_ARCH=mips64
     TARGET_BASE_ARCH=mips
     echo "TARGET_ABI_MIPSN64=y" >> $config_target_mak
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 3fa85b0..8f41952 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -11,6 +11,8 @@
 #include "exec/cpu-defs.h"
 #include "fpu/softfloat.h"
 
+#define TCG_GUEST_DEFAULT_MO (0)
+
 struct CPUMIPSState;
 
 typedef struct CPUMIPSTLBContext CPUMIPSTLBContext;
-- 
2.7.4