[v2,09/11] target/mips/op_helper: hold BQL before calling cpu_mips_get_count

Message ID 20170302195337.31558-10-alex.bennee@linaro.org
State New
Headers show
Series
  • MTTCG fixups for 2.9
Related show

Commit Message

Alex Bennée March 2, 2017, 7:53 p.m.
We should hold the BQL before we transition to HW emulation. This is
because all HW emulation needs to be serialised under MTTCG
conditions. This is picked up by asserts that fire when
cpu_mips_get_count triggers and IRQ.

Reported-by: Yongbok Kim <yongbok.kim@imgtec.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 target/mips/op_helper.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

-- 
2.11.0

Comments

Yongbok Kim March 3, 2017, 11:18 a.m. | #1
On 02/03/2017 19:53, Alex Bennée wrote:
> We should hold the BQL before we transition to HW emulation. This is

> because all HW emulation needs to be serialised under MTTCG

> conditions. This is picked up by asserts that fire when

> cpu_mips_get_count triggers and IRQ.

> 

> Reported-by: Yongbok Kim <yongbok.kim@imgtec.com>

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


Hi Alex,

Thanks for this but it is required to have few more locks.
I will send the patch shortly. Please merge it into your patchset.

Regards,
Yongbok
Alex Bennée March 3, 2017, 12:54 p.m. | #2
Yongbok Kim <yongbok.kim@imgtec.com> writes:

> On 02/03/2017 19:53, Alex Bennée wrote:

>> We should hold the BQL before we transition to HW emulation. This is

>> because all HW emulation needs to be serialised under MTTCG

>> conditions. This is picked up by asserts that fire when

>> cpu_mips_get_count triggers and IRQ.

>>

>> Reported-by: Yongbok Kim <yongbok.kim@imgtec.com>

>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>

> Hi Alex,

>

> Thanks for this but it is required to have few more locks.

> I will send the patch shortly. Please merge it into your patchset.


Thanks, I'll look out for it.

Have you been tracking the MTTCG work? Once the BQL issues are sorted as
long as atomics, barriers and tlb flushes are updated you can turn on
mttcg by default.

>

> Regards,

> Yongbok



--
Alex Bennée
Yongbok Kim March 3, 2017, 1 p.m. | #3
On 03/03/2017 12:54, Alex Bennée wrote:
> 

> Yongbok Kim <yongbok.kim@imgtec.com> writes:

> 

>> On 02/03/2017 19:53, Alex Bennée wrote:

>>> We should hold the BQL before we transition to HW emulation. This is

>>> because all HW emulation needs to be serialised under MTTCG

>>> conditions. This is picked up by asserts that fire when

>>> cpu_mips_get_count triggers and IRQ.

>>>

>>> Reported-by: Yongbok Kim <yongbok.kim@imgtec.com>

>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>>

>> Hi Alex,

>>

>> Thanks for this but it is required to have few more locks.

>> I will send the patch shortly. Please merge it into your patchset.

> 

> Thanks, I'll look out for it.

> 

> Have you been tracking the MTTCG work? Once the BQL issues are sorted as

> long as atomics, barriers and tlb flushes are updated you can turn on

> mttcg by default.

> 

> 


Yes I have MIPS patches for MTTCG but it has not been rebased to the latest
version yet. I will send it after 2.9.

Regards,
Yongbok

Patch hide | download patch | download mbox

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index b683fcb025..38bca03f52 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 "qemu/host-utils.h"
 #include "exec/helper-proto.h"
@@ -827,7 +828,13 @@  target_ulong helper_mftc0_tcschefback(CPUMIPSState *env)
 
 target_ulong helper_mfc0_count(CPUMIPSState *env)
 {
-    return (int32_t)cpu_mips_get_count(env);
+    int32_t count;
+
+    qemu_mutex_lock_iothread();
+    count = (int32_t)cpu_mips_get_count(env);
+    qemu_mutex_unlock_iothread();
+
+    return count;
 }
 
 target_ulong helper_mftc0_entryhi(CPUMIPSState *env)
@@ -2296,12 +2303,16 @@  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
-    return env->CP0_Count;
+    count = env->CP0_Count;
 #else
-    return (int32_t)cpu_mips_get_count(env);
+    qemu_mutex_lock_iothread();
+    count = (int32_t)cpu_mips_get_count(env);
+    qemu_mutex_unlock_iothread();
 #endif
+    return count;
 }
 
 target_ulong helper_rdhwr_ccres(CPUMIPSState *env)