* [PATCH v8 08/25] tcg: drop global lock during TCG code execution
[not found] <20170127103505.18606-1-alex.bennee@linaro.org>
@ 2017-01-27 10:34 ` Alex Bennée
0 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:34 UTC (permalink / raw)
Cc: Jan Kiszka, KONRAD Frederic, Emilio G . Cota, Alex Bennée,
Paolo Bonzini, Peter Crosthwaite, Richard Henderson,
Michael S. Tsirkin, Eduardo Habkost, Peter Maydell, David Gibson,
Alexander Graf, open list:Overall, open list:ARM cores,
open list:PowerPC
From: Jan Kiszka <jan.kiszka@siemens.com>
This finally allows TCG to benefit from the iothread introduction: Drop
the global mutex while running pure TCG CPU code. Reacquire the lock
when entering MMIO or PIO emulation, or when leaving the TCG loop.
We have to revert a few optimization for the current TCG threading
model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not
kicking it in qemu_cpu_kick. We also need to disable RAM block
reordering until we have a more efficient locking mechanism at hand.
Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.
These numbers demonstrate where we gain something:
20338 jan 20 0 331m 75m 6904 R 99 0.9 0:50.95 qemu-system-arm
20337 jan 20 0 331m 75m 6904 S 20 0.9 0:26.50 qemu-system-arm
The guest CPU was fully loaded, but the iothread could still run mostly
independent on a second core. Without the patch we don't get beyond
32206 jan 20 0 330m 73m 7036 R 82 0.9 1:06.00 qemu-system-arm
32204 jan 20 0 330m 73m 7036 S 21 0.9 0:17.03 qemu-system-arm
We don't benefit significantly, though, when the guest is not fully
loading a host CPU.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>
[FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
[EGC: fixed iothread lock for cpu-exec IRQ handling]
Signed-off-by: Emilio G. Cota <cota@braap.org>
[AJB: -smp single-threaded fix, clean commit msg, BQL fixes]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
v8:
- merged in BQL fixes for PPC target: ppc_set_irq
- merged in BQL fixes for ARM target: ARM_CP_IO helpers
- merged in BQL fixes for ARM target: arm_call_el_change_hook
v5 (ajb, base patches):
- added an assert to BQL unlock/lock functions instead of hanging
- ensure all cpu->interrupt_requests *modifications* protected by BQL
- add a re-read on cpu->interrupt_request for correctness
- BQL fixes for:
- assert BQL held for PPC hypercalls (emulate_spar_hypercall)
- SCLP service calls on s390x
- merge conflict with kick timer patch
v4 (ajb, base patches):
- protect cpu->interrupt updates with BQL
- fix wording io_mem_notdirty calls
- s/we/with/
v3 (ajb, base-patches):
- stale iothread_unlocks removed (cpu_exit/resume_from_signal deals
with it in the longjmp).
- fix re-base conflicts
v2 (ajb):
- merge with tcg: grab iothread lock in cpu-exec interrupt handling
- use existing fns for tracking lock state
- lock iothread for mem_region
- add assert on mem region modification
- ensure smm_helper holds iothread
- Add JK s-o-b
- Fix-up FK s-o-b annotation
v1 (ajb, base-patches):
- SMP failure now fixed by previous commit
Changes from Fred Konrad (mttcg-v7 via paolo):
* Rebase on the current HEAD.
* Fixes a deadlock in qemu_devices_reset().
* Remove the mutex in address_space_*
---
cpu-exec.c | 20 ++++++++++++++++++--
cpus.c | 28 +++++-----------------------
cputlb.c | 21 ++++++++++++++++++++-
exec.c | 12 +++++++++---
hw/core/irq.c | 1 +
hw/i386/kvmvapic.c | 4 ++--
hw/intc/arm_gicv3_cpuif.c | 3 +++
hw/ppc/ppc.c | 16 +++++++++++++++-
hw/ppc/spapr.c | 3 +++
include/qom/cpu.h | 1 +
memory.c | 2 ++
qom/cpu.c | 10 ++++++++++
target/arm/helper.c | 6 ++++++
target/arm/op_helper.c | 43 +++++++++++++++++++++++++++++++++++++++----
target/i386/smm_helper.c | 7 +++++++
target/s390x/misc_helper.c | 5 ++++-
translate-all.c | 9 +++++++--
translate-common.c | 21 +++++++++++----------
18 files changed, 163 insertions(+), 49 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index f9e836c8dd..f42a128bdf 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -29,6 +29,7 @@
#include "qemu/rcu.h"
#include "exec/tb-hash.h"
#include "exec/log.h"
+#include "qemu/main-loop.h"
#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
#include "hw/i386/apic.h"
#endif
@@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)
if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
&& replay_interrupt()) {
X86CPU *x86_cpu = X86_CPU(cpu);
+ qemu_mutex_lock_iothread();
apic_poll_irq(x86_cpu->apic_state);
cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
+ qemu_mutex_unlock_iothread();
}
#endif
if (!cpu_has_work(cpu)) {
@@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
#else
if (replay_exception()) {
CPUClass *cc = CPU_GET_CLASS(cpu);
+ qemu_mutex_lock_iothread();
cc->do_interrupt(cpu);
+ qemu_mutex_unlock_iothread();
cpu->exception_index = -1;
} else if (!replay_has_interrupt()) {
/* give a chance to iothread in replay mode */
@@ -469,9 +474,11 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
TranslationBlock **last_tb)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
- int interrupt_request = cpu->interrupt_request;
- if (unlikely(interrupt_request)) {
+ if (unlikely(atomic_read(&cpu->interrupt_request))) {
+ int interrupt_request;
+ qemu_mutex_lock_iothread();
+ interrupt_request = cpu->interrupt_request;
if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
/* Mask out external interrupts for this step. */
interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
@@ -526,7 +533,12 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
the program flow was changed */
*last_tb = NULL;
}
+
+ /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */
+ qemu_mutex_unlock_iothread();
}
+
+
if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {
atomic_set(&cpu->exit_request, 0);
cpu->exception_index = EXCP_INTERRUPT;
@@ -656,8 +668,12 @@ int cpu_exec(CPUState *cpu)
g_assert(cpu == current_cpu);
g_assert(cc == CPU_GET_CLASS(cpu));
#endif /* buggy compiler */
+
cpu->can_do_io = 1;
tb_lock_reset();
+ if (qemu_mutex_iothread_locked()) {
+ qemu_mutex_unlock_iothread();
+ }
}
} /* for(;;) */
diff --git a/cpus.c b/cpus.c
index 6d64199831..c48bc8d5b3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1026,8 +1026,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)
#endif /* _WIN32 */
static QemuMutex qemu_global_mutex;
-static QemuCond qemu_io_proceeded_cond;
-static unsigned iothread_requesting_mutex;
static QemuThread io_thread;
@@ -1041,7 +1039,6 @@ void qemu_init_cpu_loop(void)
qemu_init_sigbus();
qemu_cond_init(&qemu_cpu_cond);
qemu_cond_init(&qemu_pause_cond);
- qemu_cond_init(&qemu_io_proceeded_cond);
qemu_mutex_init(&qemu_global_mutex);
qemu_thread_get_self(&io_thread);
@@ -1084,10 +1081,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
start_tcg_kick_timer();
- while (iothread_requesting_mutex) {
- qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);
- }
-
CPU_FOREACH(cpu) {
qemu_wait_io_event_common(cpu);
}
@@ -1248,9 +1241,11 @@ static int tcg_cpu_exec(CPUState *cpu)
cpu->icount_decr.u16.low = decr;
cpu->icount_extra = count;
}
+ qemu_mutex_unlock_iothread();
cpu_exec_start(cpu);
ret = cpu_exec(cpu);
cpu_exec_end(cpu);
+ qemu_mutex_lock_iothread();
#ifdef CONFIG_PROFILER
tcg_time += profile_getclock() - ti;
#endif
@@ -1478,27 +1473,14 @@ bool qemu_mutex_iothread_locked(void)
void qemu_mutex_lock_iothread(void)
{
- atomic_inc(&iothread_requesting_mutex);
- /* In the simple case there is no need to bump the VCPU thread out of
- * TCG code execution.
- */
- if (!tcg_enabled() || qemu_in_vcpu_thread() ||
- !first_cpu || !first_cpu->created) {
- qemu_mutex_lock(&qemu_global_mutex);
- atomic_dec(&iothread_requesting_mutex);
- } else {
- if (qemu_mutex_trylock(&qemu_global_mutex)) {
- qemu_cpu_kick_rr_cpu();
- qemu_mutex_lock(&qemu_global_mutex);
- }
- atomic_dec(&iothread_requesting_mutex);
- qemu_cond_broadcast(&qemu_io_proceeded_cond);
- }
+ g_assert(!qemu_mutex_iothread_locked());
+ qemu_mutex_lock(&qemu_global_mutex);
iothread_locked = true;
}
void qemu_mutex_unlock_iothread(void)
{
+ g_assert(qemu_mutex_iothread_locked());
iothread_locked = false;
qemu_mutex_unlock(&qemu_global_mutex);
}
diff --git a/cputlb.c b/cputlb.c
index 6c39927455..1cc9d9da51 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/exec-all.h"
#include "exec/memory.h"
@@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
hwaddr physaddr = iotlbentry->addr;
MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
uint64_t val;
+ bool locked = false;
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
cpu->mem_io_pc = retaddr;
@@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
}
cpu->mem_io_vaddr = addr;
+
+ if (mr->global_locking) {
+ qemu_mutex_lock_iothread();
+ locked = true;
+ }
memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);
+ if (locked) {
+ qemu_mutex_unlock_iothread();
+ }
+
return val;
}
@@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
CPUState *cpu = ENV_GET_CPU(env);
hwaddr physaddr = iotlbentry->addr;
MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
+ bool locked = false;
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {
cpu_io_recompile(cpu, retaddr);
}
-
cpu->mem_io_vaddr = addr;
cpu->mem_io_pc = retaddr;
+
+ if (mr->global_locking) {
+ qemu_mutex_lock_iothread();
+ locked = true;
+ }
memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);
+ if (locked) {
+ qemu_mutex_unlock_iothread();
+ }
}
/* Return true if ADDR is present in the victim tlb, and has been copied
diff --git a/exec.c b/exec.c
index f2bed92b64..87cf0db91e 100644
--- a/exec.c
+++ b/exec.c
@@ -2133,9 +2133,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
}
cpu->watchpoint_hit = wp;
- /* The tb_lock will be reset when cpu_loop_exit or
- * cpu_loop_exit_noexc longjmp back into the cpu_exec
- * main loop.
+ /* Both tb_lock and iothread_mutex will be reset when
+ * cpu_loop_exit or cpu_loop_exit_noexc longjmp
+ * back into the cpu_exec main loop.
*/
tb_lock();
tb_check_watchpoint(cpu);
@@ -2370,8 +2370,14 @@ static void io_mem_init(void)
memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);
memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
NULL, UINT64_MAX);
+
+ /* io_mem_notdirty calls tb_invalidate_phys_page_fast,
+ * which can be called without the iothread mutex.
+ */
memory_region_init_io(&io_mem_notdirty, NULL, ¬dirty_mem_ops, NULL,
NULL, UINT64_MAX);
+ memory_region_clear_global_locking(&io_mem_notdirty);
+
memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,
NULL, UINT64_MAX);
}
diff --git a/hw/core/irq.c b/hw/core/irq.c
index 49ff2e64fe..b98d1d69f5 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "qemu-common.h"
#include "hw/irq.h"
#include "qom/object.h"
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 702e281dc8..b3c49b2c61 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -451,8 +451,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
resume_all_vcpus();
if (!kvm_enabled()) {
- /* tb_lock will be reset when cpu_loop_exit_noexc longjmps
- * back into the cpu_exec loop. */
+ /* Both tb_lock and iothread_mutex will be reset when
+ * longjmps back into the cpu_exec loop. */
tb_lock();
tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
cpu_loop_exit_noexc(cs);
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index a9ee7fddf9..2624d8d909 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "qemu/bitops.h"
+#include "qemu/main-loop.h"
#include "trace.h"
#include "gicv3_internal.h"
#include "cpu.h"
@@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
ARMCPU *cpu = ARM_CPU(cs->cpu);
CPUARMState *env = &cpu->env;
+ g_assert(qemu_mutex_iothread_locked());
+
trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,
cs->hppi.grp, cs->hppi.prio);
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 8945869009..59c3faa6c8 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
{
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
- unsigned int old_pending = env->pending_interrupts;
+ unsigned int old_pending;
+ bool locked = false;
+
+ /* We may already have the BQL if coming from the reset path */
+ if (!qemu_mutex_iothread_locked()) {
+ locked = true;
+ qemu_mutex_lock_iothread();
+ }
+
+ old_pending = env->pending_interrupts;
if (level) {
env->pending_interrupts |= 1 << n_IRQ;
@@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
#endif
}
+
LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
"req %08x\n", __func__, env, n_IRQ, level,
env->pending_interrupts, CPU(cpu)->interrupt_request);
+
+ if (locked) {
+ qemu_mutex_unlock_iothread();
+ }
}
/* PowerPC 6xx / 7xx internal IRQ controller */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a642e663d4..745743d64b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1012,6 +1012,9 @@ static void emulate_spapr_hypercall(PowerPCCPU *cpu)
{
CPUPPCState *env = &cpu->env;
+ /* The TCG path should also be holding the BQL at this point */
+ g_assert(qemu_mutex_iothread_locked());
+
if (msr_pr) {
hcall_dprintf("Hypercall made with MSR[PR]=1\n");
env->gpr[3] = H_PRIVILEGE;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 11db2015a4..1a06ae5938 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -325,6 +325,7 @@ struct CPUState {
bool unplug;
bool crash_occurred;
bool exit_request;
+ /* updates protected by BQL */
uint32_t interrupt_request;
int singlestep_enabled;
int64_t icount_extra;
diff --git a/memory.c b/memory.c
index 2bfc37f65c..7d7b285e41 100644
--- a/memory.c
+++ b/memory.c
@@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)
AddressSpace *as;
assert(memory_region_transaction_depth);
+ assert(qemu_mutex_iothread_locked());
+
--memory_region_transaction_depth;
if (!memory_region_transaction_depth) {
if (memory_region_update_pending) {
diff --git a/qom/cpu.c b/qom/cpu.c
index 7f575879f6..bd77c05cd0 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
}
+/* Resetting the IRQ comes from across the code base so we take the
+ * BQL here if we need to. cpu_interrupt assumes it is held.*/
void cpu_reset_interrupt(CPUState *cpu, int mask)
{
+ bool need_lock = !qemu_mutex_iothread_locked();
+
+ if (need_lock) {
+ qemu_mutex_lock_iothread();
+ }
cpu->interrupt_request &= ~mask;
+ if (need_lock) {
+ qemu_mutex_unlock_iothread();
+ }
}
void cpu_exit(CPUState *cpu)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7111c8cf18..84d789be93 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6693,6 +6693,12 @@ void arm_cpu_do_interrupt(CPUState *cs)
arm_cpu_do_interrupt_aarch32(cs);
}
+ /* Hooks may change global state so BQL should be held, also the
+ * BQL needs to be held for any modification of
+ * cs->interrupt_request.
+ */
+ g_assert(qemu_mutex_iothread_locked());
+
arm_call_el_change_hook(cpu);
if (!kvm_enabled()) {
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index ba796d898e..e1a883c595 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
#include "qemu/log.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/helper-proto.h"
#include "internals.h"
@@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)
*/
env->regs[15] &= (env->thumb ? ~1 : ~3);
+ qemu_mutex_lock_iothread();
arm_call_el_change_hook(arm_env_get_cpu(env));
+ qemu_mutex_unlock_iothread();
}
/* Access to user mode registers from privileged modes. */
@@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
{
const ARMCPRegInfo *ri = rip;
- ri->writefn(env, ri, value);
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ ri->writefn(env, ri, value);
+ qemu_mutex_unlock_iothread();
+ } else {
+ ri->writefn(env, ri, value);
+ }
}
uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)
{
const ARMCPRegInfo *ri = rip;
+ uint32_t res;
- return ri->readfn(env, ri);
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ res = ri->readfn(env, ri);
+ qemu_mutex_unlock_iothread();
+ } else {
+ res = ri->readfn(env, ri);
+ }
+
+ return res;
}
void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)
{
const ARMCPRegInfo *ri = rip;
- ri->writefn(env, ri, value);
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ ri->writefn(env, ri, value);
+ qemu_mutex_unlock_iothread();
+ } else {
+ ri->writefn(env, ri, value);
+ }
}
uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)
{
const ARMCPRegInfo *ri = rip;
+ uint64_t res;
+
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ res = ri->readfn(env, ri);
+ qemu_mutex_unlock_iothread();
+ } else {
+ res = ri->readfn(env, ri);
+ }
- return ri->readfn(env, ri);
+ return res;
}
void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
@@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)
cur_el, new_el, env->pc);
}
+ qemu_mutex_lock_iothread();
arm_call_el_change_hook(arm_env_get_cpu(env));
+ qemu_mutex_unlock_iothread();
return;
diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c
index 4dd6a2c544..f051a77c4a 100644
--- a/target/i386/smm_helper.c
+++ b/target/i386/smm_helper.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/helper-proto.h"
#include "exec/log.h"
@@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)
#define SMM_REVISION_ID 0x00020000
#endif
+/* Called with iothread lock taken */
void cpu_smm_update(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
bool smm_enabled = (env->hflags & HF_SMM_MASK);
+ g_assert(qemu_mutex_iothread_locked());
+
if (cpu->smram) {
memory_region_set_enabled(cpu->smram, smm_enabled);
}
@@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)
}
env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;
env->hflags &= ~HF_SMM_MASK;
+
+ qemu_mutex_lock_iothread();
cpu_smm_update(cpu);
+ qemu_mutex_unlock_iothread();
qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index c9604ea9c7..3cb942e8bb 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -25,6 +25,7 @@
#include "exec/helper-proto.h"
#include "sysemu/kvm.h"
#include "qemu/timer.h"
+#include "qemu/main-loop.h"
#include "exec/address-spaces.h"
#ifdef CONFIG_KVM
#include <linux/kvm.h>
@@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
/* SCLP service call */
uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
{
+ qemu_mutex_lock_iothread();
int r = sclp_service_call(env, r1, r2);
if (r < 0) {
program_interrupt(env, -r, 4);
- return 0;
+ r = 0;
}
+ qemu_mutex_unlock_iothread();
return r;
}
diff --git a/translate-all.c b/translate-all.c
index 055436a676..41b36f04c6 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -55,6 +55,7 @@
#include "translate-all.h"
#include "qemu/bitmap.h"
#include "qemu/timer.h"
+#include "qemu/main-loop.h"
#include "exec/log.h"
/* #define DEBUG_TB_INVALIDATE */
@@ -1521,7 +1522,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
#ifdef CONFIG_SOFTMMU
/* len must be <= 8 and start must be a multiple of len.
* Called via softmmu_template.h when code areas are written to with
- * tb_lock held.
+ * iothread mutex not held.
*/
void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
{
@@ -1723,7 +1724,10 @@ void tb_check_watchpoint(CPUState *cpu)
#ifndef CONFIG_USER_ONLY
/* in deterministic execution mode, instructions doing device I/Os
- must be at the end of the TB */
+ * must be at the end of the TB.
+ *
+ * Called by softmmu_template.h, with iothread mutex not held.
+ */
void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
{
#if defined(TARGET_MIPS) || defined(TARGET_SH4)
@@ -1935,6 +1939,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
void cpu_interrupt(CPUState *cpu, int mask)
{
+ g_assert(qemu_mutex_iothread_locked());
cpu->interrupt_request |= mask;
cpu->tcg_exit_req = 1;
}
diff --git a/translate-common.c b/translate-common.c
index 5e989cdf70..d504dd0d33 100644
--- a/translate-common.c
+++ b/translate-common.c
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "qom/cpu.h"
#include "sysemu/cpus.h"
+#include "qemu/main-loop.h"
uintptr_t qemu_real_host_page_size;
intptr_t qemu_real_host_page_mask;
@@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;
static void tcg_handle_interrupt(CPUState *cpu, int mask)
{
int old_mask;
+ g_assert(qemu_mutex_iothread_locked());
old_mask = cpu->interrupt_request;
cpu->interrupt_request |= mask;
@@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)
*/
if (!qemu_cpu_is_self(cpu)) {
qemu_cpu_kick(cpu);
- return;
- }
-
- if (use_icount) {
- cpu->icount_decr.u16.high = 0xffff;
- if (!cpu->can_do_io
- && (mask & ~old_mask) != 0) {
- cpu_abort(cpu, "Raised interrupt while not in I/O function");
- }
} else {
- cpu->tcg_exit_req = 1;
+ if (use_icount) {
+ cpu->icount_decr.u16.high = 0xffff;
+ if (!cpu->can_do_io
+ && (mask & ~old_mask) != 0) {
+ cpu_abort(cpu, "Raised interrupt while not in I/O function");
+ }
+ } else {
+ cpu->tcg_exit_req = 1;
+ }
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 08/25] tcg: drop global lock during TCG code execution
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-29 21:33 ` Pranith Kumar
2017-01-27 10:39 ` [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap Alex Bennée
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
Peter Crosthwaite, Michael S. Tsirkin, Eduardo Habkost,
David Gibson, Alexander Graf, open list:ARM cores,
open list:PowerPC
From: Jan Kiszka <jan.kiszka@siemens.com>
This finally allows TCG to benefit from the iothread introduction: Drop
the global mutex while running pure TCG CPU code. Reacquire the lock
when entering MMIO or PIO emulation, or when leaving the TCG loop.
We have to revert a few optimization for the current TCG threading
model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not
kicking it in qemu_cpu_kick. We also need to disable RAM block
reordering until we have a more efficient locking mechanism at hand.
Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.
These numbers demonstrate where we gain something:
20338 jan 20 0 331m 75m 6904 R 99 0.9 0:50.95 qemu-system-arm
20337 jan 20 0 331m 75m 6904 S 20 0.9 0:26.50 qemu-system-arm
The guest CPU was fully loaded, but the iothread could still run mostly
independent on a second core. Without the patch we don't get beyond
32206 jan 20 0 330m 73m 7036 R 82 0.9 1:06.00 qemu-system-arm
32204 jan 20 0 330m 73m 7036 S 21 0.9 0:17.03 qemu-system-arm
We don't benefit significantly, though, when the guest is not fully
loading a host CPU.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>
[FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
[EGC: fixed iothread lock for cpu-exec IRQ handling]
Signed-off-by: Emilio G. Cota <cota@braap.org>
[AJB: -smp single-threaded fix, clean commit msg, BQL fixes]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
v8:
- merged in BQL fixes for PPC target: ppc_set_irq
- merged in BQL fixes for ARM target: ARM_CP_IO helpers
- merged in BQL fixes for ARM target: arm_call_el_change_hook
v5 (ajb, base patches):
- added an assert to BQL unlock/lock functions instead of hanging
- ensure all cpu->interrupt_requests *modifications* protected by BQL
- add a re-read on cpu->interrupt_request for correctness
- BQL fixes for:
- assert BQL held for PPC hypercalls (emulate_spar_hypercall)
- SCLP service calls on s390x
- merge conflict with kick timer patch
v4 (ajb, base patches):
- protect cpu->interrupt updates with BQL
- fix wording io_mem_notdirty calls
- s/we/with/
v3 (ajb, base-patches):
- stale iothread_unlocks removed (cpu_exit/resume_from_signal deals
with it in the longjmp).
- fix re-base conflicts
v2 (ajb):
- merge with tcg: grab iothread lock in cpu-exec interrupt handling
- use existing fns for tracking lock state
- lock iothread for mem_region
- add assert on mem region modification
- ensure smm_helper holds iothread
- Add JK s-o-b
- Fix-up FK s-o-b annotation
v1 (ajb, base-patches):
- SMP failure now fixed by previous commit
Changes from Fred Konrad (mttcg-v7 via paolo):
* Rebase on the current HEAD.
* Fixes a deadlock in qemu_devices_reset().
* Remove the mutex in address_space_*
---
cpu-exec.c | 20 ++++++++++++++++++--
cpus.c | 28 +++++-----------------------
cputlb.c | 21 ++++++++++++++++++++-
exec.c | 12 +++++++++---
hw/core/irq.c | 1 +
hw/i386/kvmvapic.c | 4 ++--
hw/intc/arm_gicv3_cpuif.c | 3 +++
hw/ppc/ppc.c | 16 +++++++++++++++-
hw/ppc/spapr.c | 3 +++
include/qom/cpu.h | 1 +
memory.c | 2 ++
qom/cpu.c | 10 ++++++++++
target/arm/helper.c | 6 ++++++
target/arm/op_helper.c | 43 +++++++++++++++++++++++++++++++++++++++----
target/i386/smm_helper.c | 7 +++++++
target/s390x/misc_helper.c | 5 ++++-
translate-all.c | 9 +++++++--
translate-common.c | 21 +++++++++++----------
18 files changed, 163 insertions(+), 49 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index f9e836c8dd..f42a128bdf 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -29,6 +29,7 @@
#include "qemu/rcu.h"
#include "exec/tb-hash.h"
#include "exec/log.h"
+#include "qemu/main-loop.h"
#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
#include "hw/i386/apic.h"
#endif
@@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)
if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
&& replay_interrupt()) {
X86CPU *x86_cpu = X86_CPU(cpu);
+ qemu_mutex_lock_iothread();
apic_poll_irq(x86_cpu->apic_state);
cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
+ qemu_mutex_unlock_iothread();
}
#endif
if (!cpu_has_work(cpu)) {
@@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
#else
if (replay_exception()) {
CPUClass *cc = CPU_GET_CLASS(cpu);
+ qemu_mutex_lock_iothread();
cc->do_interrupt(cpu);
+ qemu_mutex_unlock_iothread();
cpu->exception_index = -1;
} else if (!replay_has_interrupt()) {
/* give a chance to iothread in replay mode */
@@ -469,9 +474,11 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
TranslationBlock **last_tb)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
- int interrupt_request = cpu->interrupt_request;
- if (unlikely(interrupt_request)) {
+ if (unlikely(atomic_read(&cpu->interrupt_request))) {
+ int interrupt_request;
+ qemu_mutex_lock_iothread();
+ interrupt_request = cpu->interrupt_request;
if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
/* Mask out external interrupts for this step. */
interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
@@ -526,7 +533,12 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
the program flow was changed */
*last_tb = NULL;
}
+
+ /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */
+ qemu_mutex_unlock_iothread();
}
+
+
if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {
atomic_set(&cpu->exit_request, 0);
cpu->exception_index = EXCP_INTERRUPT;
@@ -656,8 +668,12 @@ int cpu_exec(CPUState *cpu)
g_assert(cpu == current_cpu);
g_assert(cc == CPU_GET_CLASS(cpu));
#endif /* buggy compiler */
+
cpu->can_do_io = 1;
tb_lock_reset();
+ if (qemu_mutex_iothread_locked()) {
+ qemu_mutex_unlock_iothread();
+ }
}
} /* for(;;) */
diff --git a/cpus.c b/cpus.c
index 6d64199831..c48bc8d5b3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1026,8 +1026,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)
#endif /* _WIN32 */
static QemuMutex qemu_global_mutex;
-static QemuCond qemu_io_proceeded_cond;
-static unsigned iothread_requesting_mutex;
static QemuThread io_thread;
@@ -1041,7 +1039,6 @@ void qemu_init_cpu_loop(void)
qemu_init_sigbus();
qemu_cond_init(&qemu_cpu_cond);
qemu_cond_init(&qemu_pause_cond);
- qemu_cond_init(&qemu_io_proceeded_cond);
qemu_mutex_init(&qemu_global_mutex);
qemu_thread_get_self(&io_thread);
@@ -1084,10 +1081,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
start_tcg_kick_timer();
- while (iothread_requesting_mutex) {
- qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);
- }
-
CPU_FOREACH(cpu) {
qemu_wait_io_event_common(cpu);
}
@@ -1248,9 +1241,11 @@ static int tcg_cpu_exec(CPUState *cpu)
cpu->icount_decr.u16.low = decr;
cpu->icount_extra = count;
}
+ qemu_mutex_unlock_iothread();
cpu_exec_start(cpu);
ret = cpu_exec(cpu);
cpu_exec_end(cpu);
+ qemu_mutex_lock_iothread();
#ifdef CONFIG_PROFILER
tcg_time += profile_getclock() - ti;
#endif
@@ -1478,27 +1473,14 @@ bool qemu_mutex_iothread_locked(void)
void qemu_mutex_lock_iothread(void)
{
- atomic_inc(&iothread_requesting_mutex);
- /* In the simple case there is no need to bump the VCPU thread out of
- * TCG code execution.
- */
- if (!tcg_enabled() || qemu_in_vcpu_thread() ||
- !first_cpu || !first_cpu->created) {
- qemu_mutex_lock(&qemu_global_mutex);
- atomic_dec(&iothread_requesting_mutex);
- } else {
- if (qemu_mutex_trylock(&qemu_global_mutex)) {
- qemu_cpu_kick_rr_cpu();
- qemu_mutex_lock(&qemu_global_mutex);
- }
- atomic_dec(&iothread_requesting_mutex);
- qemu_cond_broadcast(&qemu_io_proceeded_cond);
- }
+ g_assert(!qemu_mutex_iothread_locked());
+ qemu_mutex_lock(&qemu_global_mutex);
iothread_locked = true;
}
void qemu_mutex_unlock_iothread(void)
{
+ g_assert(qemu_mutex_iothread_locked());
iothread_locked = false;
qemu_mutex_unlock(&qemu_global_mutex);
}
diff --git a/cputlb.c b/cputlb.c
index 6c39927455..1cc9d9da51 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/exec-all.h"
#include "exec/memory.h"
@@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
hwaddr physaddr = iotlbentry->addr;
MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
uint64_t val;
+ bool locked = false;
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
cpu->mem_io_pc = retaddr;
@@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
}
cpu->mem_io_vaddr = addr;
+
+ if (mr->global_locking) {
+ qemu_mutex_lock_iothread();
+ locked = true;
+ }
memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);
+ if (locked) {
+ qemu_mutex_unlock_iothread();
+ }
+
return val;
}
@@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
CPUState *cpu = ENV_GET_CPU(env);
hwaddr physaddr = iotlbentry->addr;
MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
+ bool locked = false;
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {
cpu_io_recompile(cpu, retaddr);
}
-
cpu->mem_io_vaddr = addr;
cpu->mem_io_pc = retaddr;
+
+ if (mr->global_locking) {
+ qemu_mutex_lock_iothread();
+ locked = true;
+ }
memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);
+ if (locked) {
+ qemu_mutex_unlock_iothread();
+ }
}
/* Return true if ADDR is present in the victim tlb, and has been copied
diff --git a/exec.c b/exec.c
index f2bed92b64..87cf0db91e 100644
--- a/exec.c
+++ b/exec.c
@@ -2133,9 +2133,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
}
cpu->watchpoint_hit = wp;
- /* The tb_lock will be reset when cpu_loop_exit or
- * cpu_loop_exit_noexc longjmp back into the cpu_exec
- * main loop.
+ /* Both tb_lock and iothread_mutex will be reset when
+ * cpu_loop_exit or cpu_loop_exit_noexc longjmp
+ * back into the cpu_exec main loop.
*/
tb_lock();
tb_check_watchpoint(cpu);
@@ -2370,8 +2370,14 @@ static void io_mem_init(void)
memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);
memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
NULL, UINT64_MAX);
+
+ /* io_mem_notdirty calls tb_invalidate_phys_page_fast,
+ * which can be called without the iothread mutex.
+ */
memory_region_init_io(&io_mem_notdirty, NULL, ¬dirty_mem_ops, NULL,
NULL, UINT64_MAX);
+ memory_region_clear_global_locking(&io_mem_notdirty);
+
memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,
NULL, UINT64_MAX);
}
diff --git a/hw/core/irq.c b/hw/core/irq.c
index 49ff2e64fe..b98d1d69f5 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "qemu-common.h"
#include "hw/irq.h"
#include "qom/object.h"
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 702e281dc8..b3c49b2c61 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -451,8 +451,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
resume_all_vcpus();
if (!kvm_enabled()) {
- /* tb_lock will be reset when cpu_loop_exit_noexc longjmps
- * back into the cpu_exec loop. */
+ /* Both tb_lock and iothread_mutex will be reset when
+ * longjmps back into the cpu_exec loop. */
tb_lock();
tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
cpu_loop_exit_noexc(cs);
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index a9ee7fddf9..2624d8d909 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "qemu/bitops.h"
+#include "qemu/main-loop.h"
#include "trace.h"
#include "gicv3_internal.h"
#include "cpu.h"
@@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
ARMCPU *cpu = ARM_CPU(cs->cpu);
CPUARMState *env = &cpu->env;
+ g_assert(qemu_mutex_iothread_locked());
+
trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,
cs->hppi.grp, cs->hppi.prio);
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 8945869009..59c3faa6c8 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
{
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
- unsigned int old_pending = env->pending_interrupts;
+ unsigned int old_pending;
+ bool locked = false;
+
+ /* We may already have the BQL if coming from the reset path */
+ if (!qemu_mutex_iothread_locked()) {
+ locked = true;
+ qemu_mutex_lock_iothread();
+ }
+
+ old_pending = env->pending_interrupts;
if (level) {
env->pending_interrupts |= 1 << n_IRQ;
@@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
#endif
}
+
LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
"req %08x\n", __func__, env, n_IRQ, level,
env->pending_interrupts, CPU(cpu)->interrupt_request);
+
+ if (locked) {
+ qemu_mutex_unlock_iothread();
+ }
}
/* PowerPC 6xx / 7xx internal IRQ controller */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a642e663d4..745743d64b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1012,6 +1012,9 @@ static void emulate_spapr_hypercall(PowerPCCPU *cpu)
{
CPUPPCState *env = &cpu->env;
+ /* The TCG path should also be holding the BQL at this point */
+ g_assert(qemu_mutex_iothread_locked());
+
if (msr_pr) {
hcall_dprintf("Hypercall made with MSR[PR]=1\n");
env->gpr[3] = H_PRIVILEGE;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 11db2015a4..1a06ae5938 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -325,6 +325,7 @@ struct CPUState {
bool unplug;
bool crash_occurred;
bool exit_request;
+ /* updates protected by BQL */
uint32_t interrupt_request;
int singlestep_enabled;
int64_t icount_extra;
diff --git a/memory.c b/memory.c
index 2bfc37f65c..7d7b285e41 100644
--- a/memory.c
+++ b/memory.c
@@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)
AddressSpace *as;
assert(memory_region_transaction_depth);
+ assert(qemu_mutex_iothread_locked());
+
--memory_region_transaction_depth;
if (!memory_region_transaction_depth) {
if (memory_region_update_pending) {
diff --git a/qom/cpu.c b/qom/cpu.c
index 7f575879f6..bd77c05cd0 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
}
+/* Resetting the IRQ comes from across the code base so we take the
+ * BQL here if we need to. cpu_interrupt assumes it is held.*/
void cpu_reset_interrupt(CPUState *cpu, int mask)
{
+ bool need_lock = !qemu_mutex_iothread_locked();
+
+ if (need_lock) {
+ qemu_mutex_lock_iothread();
+ }
cpu->interrupt_request &= ~mask;
+ if (need_lock) {
+ qemu_mutex_unlock_iothread();
+ }
}
void cpu_exit(CPUState *cpu)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7111c8cf18..84d789be93 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6693,6 +6693,12 @@ void arm_cpu_do_interrupt(CPUState *cs)
arm_cpu_do_interrupt_aarch32(cs);
}
+ /* Hooks may change global state so BQL should be held, also the
+ * BQL needs to be held for any modification of
+ * cs->interrupt_request.
+ */
+ g_assert(qemu_mutex_iothread_locked());
+
arm_call_el_change_hook(cpu);
if (!kvm_enabled()) {
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index ba796d898e..e1a883c595 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
#include "qemu/log.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/helper-proto.h"
#include "internals.h"
@@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)
*/
env->regs[15] &= (env->thumb ? ~1 : ~3);
+ qemu_mutex_lock_iothread();
arm_call_el_change_hook(arm_env_get_cpu(env));
+ qemu_mutex_unlock_iothread();
}
/* Access to user mode registers from privileged modes. */
@@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
{
const ARMCPRegInfo *ri = rip;
- ri->writefn(env, ri, value);
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ ri->writefn(env, ri, value);
+ qemu_mutex_unlock_iothread();
+ } else {
+ ri->writefn(env, ri, value);
+ }
}
uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)
{
const ARMCPRegInfo *ri = rip;
+ uint32_t res;
- return ri->readfn(env, ri);
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ res = ri->readfn(env, ri);
+ qemu_mutex_unlock_iothread();
+ } else {
+ res = ri->readfn(env, ri);
+ }
+
+ return res;
}
void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)
{
const ARMCPRegInfo *ri = rip;
- ri->writefn(env, ri, value);
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ ri->writefn(env, ri, value);
+ qemu_mutex_unlock_iothread();
+ } else {
+ ri->writefn(env, ri, value);
+ }
}
uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)
{
const ARMCPRegInfo *ri = rip;
+ uint64_t res;
+
+ if (ri->type & ARM_CP_IO) {
+ qemu_mutex_lock_iothread();
+ res = ri->readfn(env, ri);
+ qemu_mutex_unlock_iothread();
+ } else {
+ res = ri->readfn(env, ri);
+ }
- return ri->readfn(env, ri);
+ return res;
}
void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
@@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)
cur_el, new_el, env->pc);
}
+ qemu_mutex_lock_iothread();
arm_call_el_change_hook(arm_env_get_cpu(env));
+ qemu_mutex_unlock_iothread();
return;
diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c
index 4dd6a2c544..f051a77c4a 100644
--- a/target/i386/smm_helper.c
+++ b/target/i386/smm_helper.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/helper-proto.h"
#include "exec/log.h"
@@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)
#define SMM_REVISION_ID 0x00020000
#endif
+/* Called with iothread lock taken */
void cpu_smm_update(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
bool smm_enabled = (env->hflags & HF_SMM_MASK);
+ g_assert(qemu_mutex_iothread_locked());
+
if (cpu->smram) {
memory_region_set_enabled(cpu->smram, smm_enabled);
}
@@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)
}
env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;
env->hflags &= ~HF_SMM_MASK;
+
+ qemu_mutex_lock_iothread();
cpu_smm_update(cpu);
+ qemu_mutex_unlock_iothread();
qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index c9604ea9c7..3cb942e8bb 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -25,6 +25,7 @@
#include "exec/helper-proto.h"
#include "sysemu/kvm.h"
#include "qemu/timer.h"
+#include "qemu/main-loop.h"
#include "exec/address-spaces.h"
#ifdef CONFIG_KVM
#include <linux/kvm.h>
@@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
/* SCLP service call */
uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
{
+ qemu_mutex_lock_iothread();
int r = sclp_service_call(env, r1, r2);
if (r < 0) {
program_interrupt(env, -r, 4);
- return 0;
+ r = 0;
}
+ qemu_mutex_unlock_iothread();
return r;
}
diff --git a/translate-all.c b/translate-all.c
index 055436a676..41b36f04c6 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -55,6 +55,7 @@
#include "translate-all.h"
#include "qemu/bitmap.h"
#include "qemu/timer.h"
+#include "qemu/main-loop.h"
#include "exec/log.h"
/* #define DEBUG_TB_INVALIDATE */
@@ -1521,7 +1522,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
#ifdef CONFIG_SOFTMMU
/* len must be <= 8 and start must be a multiple of len.
* Called via softmmu_template.h when code areas are written to with
- * tb_lock held.
+ * iothread mutex not held.
*/
void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
{
@@ -1723,7 +1724,10 @@ void tb_check_watchpoint(CPUState *cpu)
#ifndef CONFIG_USER_ONLY
/* in deterministic execution mode, instructions doing device I/Os
- must be at the end of the TB */
+ * must be at the end of the TB.
+ *
+ * Called by softmmu_template.h, with iothread mutex not held.
+ */
void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
{
#if defined(TARGET_MIPS) || defined(TARGET_SH4)
@@ -1935,6 +1939,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
void cpu_interrupt(CPUState *cpu, int mask)
{
+ g_assert(qemu_mutex_iothread_locked());
cpu->interrupt_request |= mask;
cpu->tcg_exit_req = 1;
}
diff --git a/translate-common.c b/translate-common.c
index 5e989cdf70..d504dd0d33 100644
--- a/translate-common.c
+++ b/translate-common.c
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "qom/cpu.h"
#include "sysemu/cpus.h"
+#include "qemu/main-loop.h"
uintptr_t qemu_real_host_page_size;
intptr_t qemu_real_host_page_mask;
@@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;
static void tcg_handle_interrupt(CPUState *cpu, int mask)
{
int old_mask;
+ g_assert(qemu_mutex_iothread_locked());
old_mask = cpu->interrupt_request;
cpu->interrupt_request |= mask;
@@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)
*/
if (!qemu_cpu_is_self(cpu)) {
qemu_cpu_kick(cpu);
- return;
- }
-
- if (use_icount) {
- cpu->icount_decr.u16.high = 0xffff;
- if (!cpu->can_do_io
- && (mask & ~old_mask) != 0) {
- cpu_abort(cpu, "Raised interrupt while not in I/O function");
- }
} else {
- cpu->tcg_exit_req = 1;
+ if (use_icount) {
+ cpu->icount_decr.u16.high = 0xffff;
+ if (!cpu->can_do_io
+ && (mask & ~old_mask) != 0) {
+ cpu_abort(cpu, "Raised interrupt while not in I/O function");
+ }
+ } else {
+ cpu->tcg_exit_req = 1;
+ }
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
2017-01-27 10:39 ` [PATCH v8 08/25] tcg: drop global lock during TCG code execution Alex Bennée
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-31 23:09 ` Richard Henderson
2017-01-27 10:39 ` [PATCH v8 20/25] target-arm/powerctl: defer cpu reset work to CPU context Alex Bennée
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
Peter Crosthwaite, Mark Cave-Ayland, Artyom Tarasenko,
open list:ARM
While the vargs approach was flexible the original MTTCG ended up
having munge the bits to a bitmap so the data could be used in
deferred work helpers. Instead of hiding that in cputlb we push the
change to the API to make it take a bitmap of MMU indexes instead.
This change is fairly mechanical but as storing the actual index is
useful for cases like the current running context. As a result the
constants are renamed to ARMMMUBit_foo and a couple of helper
functions added to convert between a single bit and a scalar index.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
cputlb.c | 60 +++++-------
include/exec/exec-all.h | 13 +--
target/arm/cpu.h | 41 +++++---
target/arm/helper.c | 227 ++++++++++++++++++++++-----------------------
target/arm/translate-a64.c | 14 +--
target/arm/translate.c | 24 +++--
target/arm/translate.h | 4 +-
target/sparc/ldst_helper.c | 8 +-
8 files changed, 194 insertions(+), 197 deletions(-)
diff --git a/cputlb.c b/cputlb.c
index 5dfd3c3ba9..97e5c12de8 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -122,26 +122,25 @@ void tlb_flush(CPUState *cpu)
}
}
-static inline void v_tlb_flush_by_mmuidx(CPUState *cpu, va_list argp)
+static inline void v_tlb_flush_by_mmuidx(CPUState *cpu, uint16_t idxmap)
{
CPUArchState *env = cpu->env_ptr;
+ unsigned long mmu_idx_bitmask = idxmap;
+ int mmu_idx;
assert_cpu_is_self(cpu);
tlb_debug("start\n");
tb_lock();
- for (;;) {
- int mmu_idx = va_arg(argp, int);
-
- if (mmu_idx < 0) {
- break;
- }
+ for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
- tlb_debug("%d\n", mmu_idx);
+ if (test_bit(mmu_idx, &mmu_idx_bitmask)) {
+ tlb_debug("%d\n", mmu_idx);
- memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
- memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
+ memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
+ memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
+ }
}
memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
@@ -149,12 +148,9 @@ static inline void v_tlb_flush_by_mmuidx(CPUState *cpu, va_list argp)
tb_unlock();
}
-void tlb_flush_by_mmuidx(CPUState *cpu, ...)
+void tlb_flush_by_mmuidx(CPUState *cpu, uint16_t idxmap)
{
- va_list argp;
- va_start(argp, cpu);
- v_tlb_flush_by_mmuidx(cpu, argp);
- va_end(argp);
+ v_tlb_flush_by_mmuidx(cpu, idxmap);
}
static inline void tlb_flush_entry(CPUTLBEntry *tlb_entry, target_ulong addr)
@@ -219,13 +215,11 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
}
}
-void tlb_flush_page_by_mmuidx(CPUState *cpu, target_ulong addr, ...)
+void tlb_flush_page_by_mmuidx(CPUState *cpu, target_ulong addr, uint16_t idxmap)
{
CPUArchState *env = cpu->env_ptr;
- int i, k;
- va_list argp;
-
- va_start(argp, addr);
+ unsigned long mmu_idx_bitmap = idxmap;
+ int i, page, mmu_idx;
assert_cpu_is_self(cpu);
tlb_debug("addr "TARGET_FMT_lx"\n", addr);
@@ -236,31 +230,23 @@ void tlb_flush_page_by_mmuidx(CPUState *cpu, target_ulong addr, ...)
TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
env->tlb_flush_addr, env->tlb_flush_mask);
- v_tlb_flush_by_mmuidx(cpu, argp);
- va_end(argp);
+ v_tlb_flush_by_mmuidx(cpu, idxmap);
return;
}
addr &= TARGET_PAGE_MASK;
- i = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-
- for (;;) {
- int mmu_idx = va_arg(argp, int);
+ page = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
- if (mmu_idx < 0) {
- break;
- }
-
- tlb_debug("idx %d\n", mmu_idx);
-
- tlb_flush_entry(&env->tlb_table[mmu_idx][i], addr);
+ for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
+ if (test_bit(mmu_idx, &mmu_idx_bitmap)) {
+ tlb_flush_entry(&env->tlb_table[mmu_idx][page], addr);
- /* check whether there are vltb entries that need to be flushed */
- for (k = 0; k < CPU_VTLB_SIZE; k++) {
- tlb_flush_entry(&env->tlb_v_table[mmu_idx][k], addr);
+ /* check whether there are vltb entries that need to be flushed */
+ for (i = 0; i < CPU_VTLB_SIZE; i++) {
+ tlb_flush_entry(&env->tlb_v_table[mmu_idx][i], addr);
+ }
}
}
- va_end(argp);
tb_flush_jmp_cache(cpu, addr);
}
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index e43cb68355..a6c17ed74a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -106,21 +106,22 @@ void tlb_flush(CPUState *cpu);
* tlb_flush_page_by_mmuidx:
* @cpu: CPU whose TLB should be flushed
* @addr: virtual address of page to be flushed
- * @...: list of MMU indexes to flush, terminated by a negative value
+ * @idxmap: bitmap of MMU indexes to flush
*
* Flush one page from the TLB of the specified CPU, for the specified
* MMU indexes.
*/
-void tlb_flush_page_by_mmuidx(CPUState *cpu, target_ulong addr, ...);
+void tlb_flush_page_by_mmuidx(CPUState *cpu, target_ulong addr,
+ uint16_t idxmap);
/**
* tlb_flush_by_mmuidx:
* @cpu: CPU whose TLB should be flushed
- * @...: list of MMU indexes to flush, terminated by a negative value
+ * @idxmap: bitmap of MMU indexes to flush
*
* Flush all entries from the TLB of the specified CPU, for the specified
* MMU indexes.
*/
-void tlb_flush_by_mmuidx(CPUState *cpu, ...);
+void tlb_flush_by_mmuidx(CPUState *cpu, uint16_t idxmap);
/**
* tlb_set_page_with_attrs:
* @cpu: CPU to add this TLB entry for
@@ -169,11 +170,11 @@ static inline void tlb_flush(CPUState *cpu)
}
static inline void tlb_flush_page_by_mmuidx(CPUState *cpu,
- target_ulong addr, ...)
+ target_ulong addr, uint16_t idxmap)
{
}
-static inline void tlb_flush_by_mmuidx(CPUState *cpu, ...)
+static inline void tlb_flush_by_mmuidx(CPUState *cpu, uint16_t idxmap)
{
}
#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 151a5d754e..274ef17562 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1954,27 +1954,40 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
* of the AT/ATS operations.
* The values used are carefully arranged to make mmu_idx => EL lookup easy.
*/
-typedef enum ARMMMUIdx {
- ARMMMUIdx_S12NSE0 = 0,
- ARMMMUIdx_S12NSE1 = 1,
- ARMMMUIdx_S1E2 = 2,
- ARMMMUIdx_S1E3 = 3,
- ARMMMUIdx_S1SE0 = 4,
- ARMMMUIdx_S1SE1 = 5,
- ARMMMUIdx_S2NS = 6,
+typedef enum ARMMMUBitMap {
+ ARMMMUBit_S12NSE0 = 1 << 0,
+ ARMMMUBit_S12NSE1 = 1 << 1,
+ ARMMMUBit_S1E2 = 1 << 2,
+ ARMMMUBit_S1E3 = 1 << 3,
+ ARMMMUBit_S1SE0 = 1 << 4,
+ ARMMMUBit_S1SE1 = 1 << 5,
+ ARMMMUBit_S2NS = 1 << 6,
/* Indexes below here don't have TLBs and are used only for AT system
* instructions or for the first stage of an S12 page table walk.
*/
- ARMMMUIdx_S1NSE0 = 7,
- ARMMMUIdx_S1NSE1 = 8,
-} ARMMMUIdx;
+ ARMMMUBit_S1NSE0 = 1 << 7,
+ ARMMMUBit_S1NSE1 = 1 << 8,
+} ARMMMUBitMap;
-#define MMU_USER_IDX 0
+typedef int ARMMMUIdx;
+
+static inline ARMMMUIdx arm_mmu_bit_to_idx(ARMMMUBitMap bit)
+{
+ g_assert(ctpop16(bit) == 1);
+ return ctz32(bit);
+}
+
+static inline ARMMMUBitMap arm_mmu_idx_to_bit(ARMMMUIdx idx)
+{
+ return 1 << idx;
+}
+
+#define MMU_USER_IDX (1 << 0)
/* Return the exception level we're running at if this is our mmu_idx */
static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
{
- assert(mmu_idx < ARMMMUIdx_S2NS);
+ assert(mmu_idx < arm_mmu_bit_to_idx(ARMMMUBit_S2NS));
return mmu_idx & 3;
}
@@ -1984,7 +1997,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool ifetch)
int el = arm_current_el(env);
if (el < 2 && arm_is_secure_below_el3(env)) {
- return ARMMMUIdx_S1SE0 + el;
+ return arm_mmu_bit_to_idx(ARMMMUBit_S1SE0) + el;
}
return el;
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 84d789be93..0c6fb4add7 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -578,8 +578,8 @@ static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = ENV_GET_CPU(env);
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0,
- ARMMMUIdx_S2NS, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S12NSE1 | ARMMMUBit_S12NSE0
+ | ARMMMUBit_S2NS);
}
static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -588,8 +588,8 @@ static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *other_cs;
CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S12NSE1,
- ARMMMUIdx_S12NSE0, ARMMMUIdx_S2NS, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
+ ARMMMUBit_S12NSE0 | ARMMMUBit_S2NS);
}
}
@@ -611,7 +611,7 @@ static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 40);
- tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdx_S2NS, -1);
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUBit_S2NS);
}
static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -627,7 +627,7 @@ static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 40);
CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUIdx_S2NS, -1);
+ tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S2NS);
}
}
@@ -636,7 +636,7 @@ static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = ENV_GET_CPU(env);
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S1E2, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S1E2);
}
static void tlbiall_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -645,7 +645,7 @@ static void tlbiall_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *other_cs;
CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S1E2, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1E2);
}
}
@@ -655,7 +655,7 @@ static void tlbimva_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = ENV_GET_CPU(env);
uint64_t pageaddr = value & ~MAKE_64BIT_MASK(0, 12);
- tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdx_S1E2, -1);
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUBit_S1E2);
}
static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -665,7 +665,7 @@ static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t pageaddr = value & ~MAKE_64BIT_MASK(0, 12);
CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUIdx_S1E2, -1);
+ tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1E2);
}
}
@@ -2100,7 +2100,7 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
{
int access_type = ri->opc2 & 1;
uint64_t par64;
- ARMMMUIdx mmu_idx;
+ ARMMMUBitMap mmu_bit;
int el = arm_current_el(env);
bool secure = arm_is_secure_below_el3(env);
@@ -2109,13 +2109,13 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
/* stage 1 current state PL1: ATS1CPR, ATS1CPW */
switch (el) {
case 3:
- mmu_idx = ARMMMUIdx_S1E3;
+ mmu_bit = ARMMMUBit_S1E3;
break;
case 2:
- mmu_idx = ARMMMUIdx_S1NSE1;
+ mmu_bit = ARMMMUBit_S1NSE1;
break;
case 1:
- mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
+ mmu_bit = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S1NSE1;
break;
default:
g_assert_not_reached();
@@ -2125,13 +2125,13 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
/* stage 1 current state PL0: ATS1CUR, ATS1CUW */
switch (el) {
case 3:
- mmu_idx = ARMMMUIdx_S1SE0;
+ mmu_bit = ARMMMUBit_S1SE0;
break;
case 2:
- mmu_idx = ARMMMUIdx_S1NSE0;
+ mmu_bit = ARMMMUBit_S1NSE0;
break;
case 1:
- mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
+ mmu_bit = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S1NSE0;
break;
default:
g_assert_not_reached();
@@ -2139,17 +2139,17 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
break;
case 4:
/* stage 1+2 NonSecure PL1: ATS12NSOPR, ATS12NSOPW */
- mmu_idx = ARMMMUIdx_S12NSE1;
+ mmu_bit = ARMMMUBit_S12NSE1;
break;
case 6:
/* stage 1+2 NonSecure PL0: ATS12NSOUR, ATS12NSOUW */
- mmu_idx = ARMMMUIdx_S12NSE0;
+ mmu_bit = ARMMMUBit_S12NSE0;
break;
default:
g_assert_not_reached();
}
- par64 = do_ats_write(env, value, access_type, mmu_idx);
+ par64 = do_ats_write(env, value, access_type, arm_mmu_bit_to_idx(mmu_bit));
A32_BANKED_CURRENT_REG_SET(env, par, par64);
}
@@ -2160,7 +2160,8 @@ static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
int access_type = ri->opc2 & 1;
uint64_t par64;
- par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS);
+ par64 = do_ats_write(env, value, access_type,
+ arm_mmu_bit_to_idx(ARMMMUBit_S2NS));
A32_BANKED_CURRENT_REG_SET(env, par, par64);
}
@@ -2185,26 +2186,26 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
case 0:
switch (ri->opc1) {
case 0: /* AT S1E1R, AT S1E1W */
- mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
+ mmu_idx = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S1NSE1;
break;
case 4: /* AT S1E2R, AT S1E2W */
- mmu_idx = ARMMMUIdx_S1E2;
+ mmu_idx = ARMMMUBit_S1E2;
break;
case 6: /* AT S1E3R, AT S1E3W */
- mmu_idx = ARMMMUIdx_S1E3;
+ mmu_idx = ARMMMUBit_S1E3;
break;
default:
g_assert_not_reached();
}
break;
case 2: /* AT S1E0R, AT S1E0W */
- mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
+ mmu_idx = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S1NSE0;
break;
case 4: /* AT S12E1R, AT S12E1W */
- mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S12NSE1;
+ mmu_idx = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S12NSE1;
break;
case 6: /* AT S12E0R, AT S12E0W */
- mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S12NSE0;
+ mmu_idx = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S12NSE0;
break;
default:
g_assert_not_reached();
@@ -2499,8 +2500,8 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
/* Accesses to VTTBR may change the VMID so we must flush the TLB. */
if (raw_read(env, ri) != value) {
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0,
- ARMMMUIdx_S2NS, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S12NSE1 | ARMMMUBit_S12NSE0 |
+ ARMMMUBit_S2NS);
raw_write(env, ri, value);
}
}
@@ -2859,9 +2860,9 @@ static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = CPU(cpu);
if (arm_is_secure_below_el3(env)) {
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S1SE1, ARMMMUIdx_S1SE0, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S1SE1 | ARMMMUBit_S1SE0);
} else {
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S12NSE1 | ARMMMUBit_S12NSE0);
}
}
@@ -2873,10 +2874,10 @@ static void tlbi_aa64_vmalle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPU_FOREACH(other_cs) {
if (sec) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S1SE1, ARMMMUIdx_S1SE0, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1SE1 | ARMMMUBit_S1SE0);
} else {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S12NSE1,
- ARMMMUIdx_S12NSE0, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
+ ARMMMUBit_S12NSE0);
}
}
}
@@ -2892,13 +2893,13 @@ static void tlbi_aa64_alle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = CPU(cpu);
if (arm_is_secure_below_el3(env)) {
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S1SE1, ARMMMUIdx_S1SE0, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S1SE1 | ARMMMUBit_S1SE0);
} else {
if (arm_feature(env, ARM_FEATURE_EL2)) {
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0,
- ARMMMUIdx_S2NS, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S12NSE1 | ARMMMUBit_S12NSE0 |
+ ARMMMUBit_S2NS);
} else {
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S12NSE1 | ARMMMUBit_S12NSE0);
}
}
}
@@ -2909,7 +2910,7 @@ static void tlbi_aa64_alle2_write(CPUARMState *env, const ARMCPRegInfo *ri,
ARMCPU *cpu = arm_env_get_cpu(env);
CPUState *cs = CPU(cpu);
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S1E2, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S1E2);
}
static void tlbi_aa64_alle3_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -2918,7 +2919,7 @@ static void tlbi_aa64_alle3_write(CPUARMState *env, const ARMCPRegInfo *ri,
ARMCPU *cpu = arm_env_get_cpu(env);
CPUState *cs = CPU(cpu);
- tlb_flush_by_mmuidx(cs, ARMMMUIdx_S1E3, -1);
+ tlb_flush_by_mmuidx(cs, ARMMMUBit_S1E3);
}
static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -2934,13 +2935,13 @@ static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPU_FOREACH(other_cs) {
if (sec) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S1SE1, ARMMMUIdx_S1SE0, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1SE1 | ARMMMUBit_S1SE0);
} else if (has_el2) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S12NSE1,
- ARMMMUIdx_S12NSE0, ARMMMUIdx_S2NS, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
+ ARMMMUBit_S12NSE0 | ARMMMUBit_S2NS);
} else {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S12NSE1,
- ARMMMUIdx_S12NSE0, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
+ ARMMMUBit_S12NSE0);
}
}
}
@@ -2951,7 +2952,7 @@ static void tlbi_aa64_alle2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *other_cs;
CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S1E2, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1E2);
}
}
@@ -2961,7 +2962,7 @@ static void tlbi_aa64_alle3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *other_cs;
CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUIdx_S1E3, -1);
+ tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1E3);
}
}
@@ -2978,11 +2979,11 @@ static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t pageaddr = sextract64(value << 12, 0, 56);
if (arm_is_secure_below_el3(env)) {
- tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdx_S1SE1,
- ARMMMUIdx_S1SE0, -1);
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUBit_S1SE1 |
+ ARMMMUBit_S1SE0);
} else {
- tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdx_S12NSE1,
- ARMMMUIdx_S12NSE0, -1);
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUBit_S12NSE1 |
+ ARMMMUBit_S12NSE0);
}
}
@@ -2997,7 +2998,7 @@ static void tlbi_aa64_vae2_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = CPU(cpu);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
- tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdx_S1E2, -1);
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUBit_S1E2);
}
static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3011,7 +3012,7 @@ static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPUState *cs = CPU(cpu);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
- tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdx_S1E3, -1);
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUBit_S1E3);
}
static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3023,11 +3024,11 @@ static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
CPU_FOREACH(other_cs) {
if (sec) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUIdx_S1SE1,
- ARMMMUIdx_S1SE0, -1);
+ tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1SE1 |
+ ARMMMUBit_S1SE0);
} else {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUIdx_S12NSE1,
- ARMMMUIdx_S12NSE0, -1);
+ tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S12NSE1 |
+ ARMMMUBit_S12NSE0);
}
}
}
@@ -3039,7 +3040,7 @@ static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t pageaddr = sextract64(value << 12, 0, 56);
CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUIdx_S1E2, -1);
+ tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1E2);
}
}
@@ -3050,7 +3051,7 @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t pageaddr = sextract64(value << 12, 0, 56);
CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUIdx_S1E3, -1);
+ tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1E3);
}
}
@@ -3073,7 +3074,7 @@ static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 48);
- tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdx_S2NS, -1);
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUBit_S2NS);
}
static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3089,7 +3090,7 @@ static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 48);
CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUIdx_S2NS, -1);
+ tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S2NS);
}
}
@@ -6709,41 +6710,33 @@ void arm_cpu_do_interrupt(CPUState *cs)
/* Return the exception level which controls this address translation regime */
static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
{
- switch (mmu_idx) {
- case ARMMMUIdx_S2NS:
- case ARMMMUIdx_S1E2:
+ ARMMMUBitMap bit = arm_mmu_idx_to_bit(mmu_idx);
+ if (bit & (ARMMMUBit_S2NS | ARMMMUBit_S1E2)) {
return 2;
- case ARMMMUIdx_S1E3:
+ } else if (bit & ARMMMUBit_S1E3) {
return 3;
- case ARMMMUIdx_S1SE0:
+ } else if (bit & ARMMMUBit_S1SE0) {
return arm_el_is_aa64(env, 3) ? 1 : 3;
- case ARMMMUIdx_S1SE1:
- case ARMMMUIdx_S1NSE0:
- case ARMMMUIdx_S1NSE1:
+ } else if (bit & (ARMMMUBit_S1SE1 | ARMMMUBit_S1NSE0 | ARMMMUBit_S1NSE1)) {
return 1;
- default:
- g_assert_not_reached();
}
+
+ g_assert_not_reached();
}
/* Return true if this address translation regime is secure */
static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
{
- switch (mmu_idx) {
- case ARMMMUIdx_S12NSE0:
- case ARMMMUIdx_S12NSE1:
- case ARMMMUIdx_S1NSE0:
- case ARMMMUIdx_S1NSE1:
- case ARMMMUIdx_S1E2:
- case ARMMMUIdx_S2NS:
+ ARMMMUBitMap bit = arm_mmu_idx_to_bit(mmu_idx);
+
+ if (bit & (ARMMMUBit_S12NSE0 | ARMMMUBit_S12NSE1 | ARMMMUBit_S1NSE0 |
+ ARMMMUBit_S1NSE1 | ARMMMUBit_S1E2 | ARMMMUBit_S2NS)) {
return false;
- case ARMMMUIdx_S1E3:
- case ARMMMUIdx_S1SE0:
- case ARMMMUIdx_S1SE1:
+ } else if (bit & (ARMMMUBit_S1E3 | ARMMMUBit_S1SE0 | ARMMMUBit_S1SE1)) {
return true;
- default:
- g_assert_not_reached();
}
+
+ g_assert_not_reached();
}
/* Return the SCTLR value which controls this address translation regime */
@@ -6756,7 +6749,7 @@ static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
static inline bool regime_translation_disabled(CPUARMState *env,
ARMMMUIdx mmu_idx)
{
- if (mmu_idx == ARMMMUIdx_S2NS) {
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S2NS)) {
return (env->cp15.hcr_el2 & HCR_VM) == 0;
}
return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0;
@@ -6771,7 +6764,7 @@ static inline bool regime_translation_big_endian(CPUARMState *env,
/* Return the TCR controlling this translation regime */
static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
{
- if (mmu_idx == ARMMMUIdx_S2NS) {
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S2NS)) {
return &env->cp15.vtcr_el2;
}
return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
@@ -6786,8 +6779,9 @@ uint32_t arm_regime_tbi0(CPUARMState *env, ARMMMUIdx mmu_idx)
/* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
* a stage 1+2 mmu index into the appropriate stage 1 mmu index.
*/
- if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
- mmu_idx += ARMMMUIdx_S1NSE0;
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE0) ||
+ mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE1)) {
+ mmu_idx += arm_mmu_bit_to_idx(ARMMMUBit_S1NSE0);
}
tcr = regime_tcr(env, mmu_idx);
@@ -6809,8 +6803,9 @@ uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx)
/* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
* a stage 1+2 mmu index into the appropriate stage 1 mmu index.
*/
- if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
- mmu_idx += ARMMMUIdx_S1NSE0;
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE0) ||
+ mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE1)) {
+ mmu_idx += arm_mmu_bit_to_idx(ARMMMUBit_S1NSE0);
}
tcr = regime_tcr(env, mmu_idx);
@@ -6827,7 +6822,7 @@ uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx)
static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
int ttbrn)
{
- if (mmu_idx == ARMMMUIdx_S2NS) {
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S2NS)) {
return env->cp15.vttbr_el2;
}
if (ttbrn == 0) {
@@ -6857,8 +6852,9 @@ static inline bool regime_using_lpae_format(CPUARMState *env,
* on whether the long or short descriptor format is in use. */
bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
{
- if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
- mmu_idx += ARMMMUIdx_S1NSE0;
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE0) ||
+ mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE1)) {
+ mmu_idx += arm_mmu_bit_to_idx(ARMMMUBit_S1NSE0);
}
return regime_using_lpae_format(env, mmu_idx);
@@ -6866,15 +6862,14 @@ bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
{
- switch (mmu_idx) {
- case ARMMMUIdx_S1SE0:
- case ARMMMUIdx_S1NSE0:
+ ARMMMUBitMap bit = arm_mmu_idx_to_bit(mmu_idx);
+
+ if (bit & (ARMMMUBit_S1SE0 | ARMMMUBit_S1NSE0)) {
return true;
- default:
- return false;
- case ARMMMUIdx_S12NSE0:
- case ARMMMUIdx_S12NSE1:
+ } else if (bit & (ARMMMUBit_S12NSE0 | ARMMMUBit_S12NSE1)) {
g_assert_not_reached();
+ } else {
+ return false;
}
}
@@ -7004,7 +6999,7 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
bool have_wxn;
int wxn = 0;
- assert(mmu_idx != ARMMMUIdx_S2NS);
+ assert(mmu_idx != ARMMMUBit_S2NS);
user_rw = simple_ap_to_rw_prot_is_user(ap, true);
if (is_user) {
@@ -7096,14 +7091,15 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
uint32_t *fsr,
ARMMMUFaultInfo *fi)
{
- if ((mmu_idx == ARMMMUIdx_S1NSE0 || mmu_idx == ARMMMUIdx_S1NSE1) &&
- !regime_translation_disabled(env, ARMMMUIdx_S2NS)) {
+ if ((mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S1NSE0) ||
+ mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S1NSE1)) &&
+ !regime_translation_disabled(env, arm_mmu_bit_to_idx(ARMMMUBit_S2NS))) {
target_ulong s2size;
hwaddr s2pa;
int s2prot;
int ret;
- ret = get_phys_addr_lpae(env, addr, 0, ARMMMUIdx_S2NS, &s2pa,
+ ret = get_phys_addr_lpae(env, addr, 0, ARMMMUBit_S2NS, &s2pa,
&txattrs, &s2prot, &s2size, fsr, fi);
if (ret) {
fi->s2addr = addr;
@@ -7546,7 +7542,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
level = 0;
addrsize = 64;
if (el > 1) {
- if (mmu_idx != ARMMMUIdx_S2NS) {
+ if (mmu_idx != ARMMMUBit_S2NS) {
tbi = extract64(tcr->raw_tcr, 20, 1);
}
} else {
@@ -7583,7 +7579,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
t0sz = extract32(tcr->raw_tcr, 0, 6);
t0sz = MIN(t0sz, 39);
t0sz = MAX(t0sz, 16);
- } else if (mmu_idx != ARMMMUIdx_S2NS) {
+ } else if (mmu_idx != ARMMMUBit_S2NS) {
/* AArch32 stage 1 translation. */
t0sz = extract32(tcr->raw_tcr, 0, 3);
} else {
@@ -7677,7 +7673,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
goto do_fault;
}
- if (mmu_idx != ARMMMUIdx_S2NS) {
+ if (mmu_idx != ARMMMUBit_S2NS) {
/* The starting level depends on the virtual address size (which can
* be up to 48 bits) and the translation granule size. It indicates
* the number of strides (stride bits at a time) needed to
@@ -7777,7 +7773,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
attrs = extract64(descriptor, 2, 10)
| (extract64(descriptor, 52, 12) << 10);
- if (mmu_idx == ARMMMUIdx_S2NS) {
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S2NS)) {
/* Stage 2 table descriptors do not include any attribute fields */
break;
}
@@ -7805,7 +7801,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
ap = extract32(attrs, 4, 2);
xn = extract32(attrs, 12, 1);
- if (mmu_idx == ARMMMUIdx_S2NS) {
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S2NS)) {
ns = true;
*prot = get_S2prot(env, ap, xn);
} else {
@@ -7834,7 +7830,7 @@ do_fault:
/* Long-descriptor format IFSR/DFSR value */
*fsr = (1 << 9) | (fault_type << 2) | level;
/* Tag the error as S2 for failed S1 PTW at S2 or ordinary S2. */
- fi->stage2 = fi->s1ptw || (mmu_idx == ARMMMUIdx_S2NS);
+ fi->stage2 = fi->s1ptw || (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S2NS));
return true;
}
@@ -8103,7 +8099,8 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
target_ulong *page_size, uint32_t *fsr,
ARMMMUFaultInfo *fi)
{
- if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
+ if (mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE0) ||
+ mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S12NSE1)) {
/* Call ourselves recursively to do the stage 1 and then stage 2
* translations.
*/
@@ -8113,17 +8110,17 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
int ret;
ret = get_phys_addr(env, address, access_type,
- mmu_idx + ARMMMUIdx_S1NSE0, &ipa, attrs,
+ mmu_idx + ARMMMUBit_S1NSE0, &ipa, attrs,
prot, page_size, fsr, fi);
/* If S1 fails or S2 is disabled, return early. */
- if (ret || regime_translation_disabled(env, ARMMMUIdx_S2NS)) {
+ if (ret || regime_translation_disabled(env, ARMMMUBit_S2NS)) {
*phys_ptr = ipa;
return ret;
}
/* S1 is done. Now do S2 translation. */
- ret = get_phys_addr_lpae(env, ipa, access_type, ARMMMUIdx_S2NS,
+ ret = get_phys_addr_lpae(env, ipa, access_type, ARMMMUBit_S2NS,
phys_ptr, attrs, &s2_prot,
page_size, fsr, fi);
fi->s2addr = ipa;
@@ -8134,7 +8131,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
/*
* For non-EL2 CPUs a stage1+stage2 translation is just stage 1.
*/
- mmu_idx += ARMMMUIdx_S1NSE0;
+ mmu_idx += arm_mmu_bit_to_idx(ARMMMUBit_S1NSE0);
}
}
@@ -8148,7 +8145,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
/* Fast Context Switch Extension. This doesn't exist at all in v8.
* In v7 and earlier it affects all stage 1 translations.
*/
- if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS
+ if (address < 0x02000000 && mmu_idx != arm_mmu_bit_to_idx(ARMMMUBit_S2NS)
&& !arm_feature(env, ARM_FEATURE_V8)) {
if (regime_el(env, mmu_idx) == 3) {
address += env->cp15.fcseidr_s;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index d0352e2045..88a4df6959 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -106,14 +106,14 @@ static inline ARMMMUIdx get_a64_user_mem_index(DisasContext *s)
/* Return the mmu_idx to use for A64 "unprivileged load/store" insns:
* if EL1, access as if EL0; otherwise access at current EL
*/
- switch (s->mmu_idx) {
- case ARMMMUIdx_S12NSE1:
- return ARMMMUIdx_S12NSE0;
- case ARMMMUIdx_S1SE1:
- return ARMMMUIdx_S1SE0;
- case ARMMMUIdx_S2NS:
+ ARMMMUBitMap bit = arm_mmu_idx_to_bit(s->mmu_idx);
+ if (bit & ARMMMUBit_S12NSE1) {
+ return ARMMMUBit_S12NSE0;
+ } else if (bit & ARMMMUBit_S1SE1) {
+ return ARMMMUBit_S1SE0;
+ } else if (bit & ARMMMUBit_S2NS) {
g_assert_not_reached();
- default:
+ } else {
return s->mmu_idx;
}
}
diff --git a/target/arm/translate.c b/target/arm/translate.c
index c9186b6195..dc67887918 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -109,19 +109,17 @@ static inline ARMMMUIdx get_a32_user_mem_index(DisasContext *s)
* if PL2, UNPREDICTABLE (we choose to implement as if PL0)
* otherwise, access as if at PL0.
*/
- switch (s->mmu_idx) {
- case ARMMMUIdx_S1E2: /* this one is UNPREDICTABLE */
- case ARMMMUIdx_S12NSE0:
- case ARMMMUIdx_S12NSE1:
- return ARMMMUIdx_S12NSE0;
- case ARMMMUIdx_S1E3:
- case ARMMMUIdx_S1SE0:
- case ARMMMUIdx_S1SE1:
- return ARMMMUIdx_S1SE0;
- case ARMMMUIdx_S2NS:
- default:
- g_assert_not_reached();
- }
+ ARMMMUBitMap bit = arm_mmu_idx_to_bit(s->mmu_idx);
+ if (bit & (ARMMMUBit_S1E2 | /* this one is UNPREDICTABLE */
+ ARMMMUBit_S12NSE0 |
+ ARMMMUBit_S12NSE1)) {
+ return arm_mmu_bit_to_idx(ARMMMUBit_S12NSE0);
+ } else if (bit & (ARMMMUBit_S1E3 |
+ ARMMMUBit_S1SE0 |
+ ARMMMUBit_S1SE1)) {
+ return arm_mmu_bit_to_idx(ARMMMUBit_S1SE0);
+ }
+ g_assert_not_reached();
}
static inline TCGv_i32 load_cpu_offset(int offset)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 285e96f087..8011b8562d 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -100,8 +100,8 @@ static inline int default_exception_el(DisasContext *s)
* exceptions can only be routed to ELs above 1, so we target the higher of
* 1 or the current EL.
*/
- return (s->mmu_idx == ARMMMUIdx_S1SE0 && s->secure_routed_to_el3)
- ? 3 : MAX(1, s->current_el);
+ return (s->mmu_idx == arm_mmu_bit_to_idx(ARMMMUBit_S1SE0)
+ && s->secure_routed_to_el3) ? 3 : MAX(1, s->current_el);
}
/* target-specific extra values for is_jmp */
diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index 2c05d6af75..57968d9143 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -1768,13 +1768,15 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
case 1:
env->dmmu.mmu_primary_context = val;
env->immu.mmu_primary_context = val;
- tlb_flush_by_mmuidx(CPU(cpu), MMU_USER_IDX, MMU_KERNEL_IDX, -1);
+ tlb_flush_by_mmuidx(CPU(cpu),
+ (1 << MMU_USER_IDX) | (1 << MMU_KERNEL_IDX));
break;
case 2:
env->dmmu.mmu_secondary_context = val;
env->immu.mmu_secondary_context = val;
- tlb_flush_by_mmuidx(CPU(cpu), MMU_USER_SECONDARY_IDX,
- MMU_KERNEL_SECONDARY_IDX, -1);
+ tlb_flush_by_mmuidx(CPU(cpu),
+ (1 << MMU_USER_SECONDARY_IDX) |
+ (1 << MMU_KERNEL_SECONDARY_IDX));
break;
default:
cpu_unassigned_access(cs, addr, true, false, 1, size);
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 20/25] target-arm/powerctl: defer cpu reset work to CPU context
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
2017-01-27 10:39 ` [PATCH v8 08/25] tcg: drop global lock during TCG code execution Alex Bennée
2017-01-27 10:39 ` [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap Alex Bennée
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-27 10:39 ` [PATCH v8 21/25] target-arm: don't generate WFE/YIELD calls for MTTCG Alex Bennée
` (4 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
open list:ARM
When switching a new vCPU on we want to complete a bunch of the setup
work before we start scheduling the vCPU thread. To do this cleanly we
defer vCPU setup to async work which will run the vCPUs execution
context as the thread is woken up. The scheduling of the work will kick
the vCPU awake.
This avoids potential races in MTTCG system emulation.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
v7
- add const to static mode_for_el[] array
- fix checkpatch long lines
---
target/arm/arm-powerctl.c | 146 ++++++++++++++++++++++++++++------------------
1 file changed, 88 insertions(+), 58 deletions(-)
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index fbb7a15daa..082788e3a4 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -48,11 +48,87 @@ CPUState *arm_get_cpu_by_id(uint64_t id)
return NULL;
}
+struct cpu_on_info {
+ uint64_t entry;
+ uint64_t context_id;
+ uint32_t target_el;
+ bool target_aa64;
+};
+
+
+static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
+ run_on_cpu_data data)
+{
+ ARMCPU *target_cpu = ARM_CPU(target_cpu_state);
+ struct cpu_on_info *info = (struct cpu_on_info *) data.host_ptr;
+
+ /* Initialize the cpu we are turning on */
+ cpu_reset(target_cpu_state);
+ target_cpu->powered_off = false;
+ target_cpu_state->halted = 0;
+
+ if (info->target_aa64) {
+ if ((info->target_el < 3) && arm_feature(&target_cpu->env,
+ ARM_FEATURE_EL3)) {
+ /*
+ * As target mode is AArch64, we need to set lower
+ * exception level (the requested level 2) to AArch64
+ */
+ target_cpu->env.cp15.scr_el3 |= SCR_RW;
+ }
+
+ if ((info->target_el < 2) && arm_feature(&target_cpu->env,
+ ARM_FEATURE_EL2)) {
+ /*
+ * As target mode is AArch64, we need to set lower
+ * exception level (the requested level 1) to AArch64
+ */
+ target_cpu->env.cp15.hcr_el2 |= HCR_RW;
+ }
+
+ target_cpu->env.pstate = aarch64_pstate_mode(info->target_el, true);
+ } else {
+ /* We are requested to boot in AArch32 mode */
+ static const uint32_t mode_for_el[] = { 0,
+ ARM_CPU_MODE_SVC,
+ ARM_CPU_MODE_HYP,
+ ARM_CPU_MODE_SVC };
+
+ cpsr_write(&target_cpu->env, mode_for_el[info->target_el], CPSR_M,
+ CPSRWriteRaw);
+ }
+
+ if (info->target_el == 3) {
+ /* Processor is in secure mode */
+ target_cpu->env.cp15.scr_el3 &= ~SCR_NS;
+ } else {
+ /* Processor is not in secure mode */
+ target_cpu->env.cp15.scr_el3 |= SCR_NS;
+ }
+
+ /* We check if the started CPU is now at the correct level */
+ assert(info->target_el == arm_current_el(&target_cpu->env));
+
+ if (info->target_aa64) {
+ target_cpu->env.xregs[0] = info->context_id;
+ target_cpu->env.thumb = false;
+ } else {
+ target_cpu->env.regs[0] = info->context_id;
+ target_cpu->env.thumb = info->entry & 1;
+ info->entry &= 0xfffffffe;
+ }
+
+ /* Start the new CPU at the requested address */
+ cpu_set_pc(target_cpu_state, info->entry);
+ g_free(info);
+}
+
int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id,
uint32_t target_el, bool target_aa64)
{
CPUState *target_cpu_state;
ARMCPU *target_cpu;
+ struct cpu_on_info *info;
DPRINTF("cpu %" PRId64 " (EL %d, %s) @ 0x%" PRIx64 " with R0 = 0x%" PRIx64
"\n", cpuid, target_el, target_aa64 ? "aarch64" : "aarch32", entry,
@@ -109,64 +185,18 @@ int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id,
return QEMU_ARM_POWERCTL_INVALID_PARAM;
}
- /* Initialize the cpu we are turning on */
- cpu_reset(target_cpu_state);
- target_cpu->powered_off = false;
- target_cpu_state->halted = 0;
-
- if (target_aa64) {
- if ((target_el < 3) && arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) {
- /*
- * As target mode is AArch64, we need to set lower
- * exception level (the requested level 2) to AArch64
- */
- target_cpu->env.cp15.scr_el3 |= SCR_RW;
- }
-
- if ((target_el < 2) && arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) {
- /*
- * As target mode is AArch64, we need to set lower
- * exception level (the requested level 1) to AArch64
- */
- target_cpu->env.cp15.hcr_el2 |= HCR_RW;
- }
-
- target_cpu->env.pstate = aarch64_pstate_mode(target_el, true);
- } else {
- /* We are requested to boot in AArch32 mode */
- static uint32_t mode_for_el[] = { 0,
- ARM_CPU_MODE_SVC,
- ARM_CPU_MODE_HYP,
- ARM_CPU_MODE_SVC };
-
- cpsr_write(&target_cpu->env, mode_for_el[target_el], CPSR_M,
- CPSRWriteRaw);
- }
-
- if (target_el == 3) {
- /* Processor is in secure mode */
- target_cpu->env.cp15.scr_el3 &= ~SCR_NS;
- } else {
- /* Processor is not in secure mode */
- target_cpu->env.cp15.scr_el3 |= SCR_NS;
- }
-
- /* We check if the started CPU is now at the correct level */
- assert(target_el == arm_current_el(&target_cpu->env));
-
- if (target_aa64) {
- target_cpu->env.xregs[0] = context_id;
- target_cpu->env.thumb = false;
- } else {
- target_cpu->env.regs[0] = context_id;
- target_cpu->env.thumb = entry & 1;
- entry &= 0xfffffffe;
- }
-
- /* Start the new CPU at the requested address */
- cpu_set_pc(target_cpu_state, entry);
-
- qemu_cpu_kick(target_cpu_state);
+ /* 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.
+ */
+ info = g_new(struct cpu_on_info, 1);
+ info->entry = entry;
+ info->context_id = context_id;
+ info->target_el = target_el;
+ info->target_aa64 = target_aa64;
+
+ async_run_on_cpu(target_cpu_state, arm_set_cpu_on_async_work,
+ RUN_ON_CPU_HOST_PTR(info));
/* We are good to go */
return QEMU_ARM_POWERCTL_RET_SUCCESS;
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 21/25] target-arm: don't generate WFE/YIELD calls for MTTCG
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
` (2 preceding siblings ...)
2017-01-27 10:39 ` [PATCH v8 20/25] target-arm/powerctl: defer cpu reset work to CPU context Alex Bennée
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-27 10:39 ` [PATCH v8 22/25] target-arm/cpu.h: make ARM_CP defined consistent Alex Bennée
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
open list:ARM
The WFE and YIELD instructions are really only hints and in TCG's case
they were useful to move the scheduling on from one vCPU to the next. In
the parallel context (MTTCG) this just causes an unnecessary cpu_exit
and contention of the BQL.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
target/arm/op_helper.c | 7 +++++++
target/arm/translate-a64.c | 8 ++++++--
target/arm/translate.c | 20 ++++++++++++++++----
3 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index e1a883c595..abfa7cdd39 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -436,6 +436,13 @@ void HELPER(yield)(CPUARMState *env)
ARMCPU *cpu = arm_env_get_cpu(env);
CPUState *cs = CPU(cpu);
+ /* When running in MTTCG we don't generate jumps to the yield and
+ * WFE helpers as it won't affect the scheduling of other vCPUs.
+ * If we wanted to more completely model WFE/SEV so we don't busy
+ * spin unnecessarily we would need to do something more involved.
+ */
+ g_assert(!parallel_cpus);
+
/* This is a non-trappable hint instruction that generally indicates
* that the guest is currently busy-looping. Yield control back to the
* top level loop so that a more deserving VCPU has a chance to run.
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 88a4df6959..05162f335e 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1342,10 +1342,14 @@ static void handle_hint(DisasContext *s, uint32_t insn,
s->is_jmp = DISAS_WFI;
return;
case 1: /* YIELD */
- s->is_jmp = DISAS_YIELD;
+ if (!parallel_cpus) {
+ s->is_jmp = DISAS_YIELD;
+ }
return;
case 2: /* WFE */
- s->is_jmp = DISAS_WFE;
+ if (!parallel_cpus) {
+ s->is_jmp = DISAS_WFE;
+ }
return;
case 4: /* SEV */
case 5: /* SEVL */
diff --git a/target/arm/translate.c b/target/arm/translate.c
index dc67887918..444a24c2b6 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -4343,20 +4343,32 @@ static void gen_exception_return(DisasContext *s, TCGv_i32 pc)
gen_rfe(s, pc, load_cpu_field(spsr));
}
+/*
+ * For WFI we will halt the vCPU until an IRQ. For WFE and YIELD we
+ * only call the helper when running single threaded TCG code to ensure
+ * the next round-robin scheduled vCPU gets a crack. In MTTCG mode we
+ * just skip this instruction. Currently the SEV/SEVL instructions
+ * which are *one* of many ways to wake the CPU from WFE are not
+ * implemented so we can't sleep like WFI does.
+ */
static void gen_nop_hint(DisasContext *s, int val)
{
switch (val) {
case 1: /* yield */
- gen_set_pc_im(s, s->pc);
- s->is_jmp = DISAS_YIELD;
+ if (!parallel_cpus) {
+ gen_set_pc_im(s, s->pc);
+ s->is_jmp = DISAS_YIELD;
+ }
break;
case 3: /* wfi */
gen_set_pc_im(s, s->pc);
s->is_jmp = DISAS_WFI;
break;
case 2: /* wfe */
- gen_set_pc_im(s, s->pc);
- s->is_jmp = DISAS_WFE;
+ if (!parallel_cpus) {
+ gen_set_pc_im(s, s->pc);
+ s->is_jmp = DISAS_WFE;
+ }
break;
case 4: /* sev */
case 5: /* sevl */
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 22/25] target-arm/cpu.h: make ARM_CP defined consistent
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
` (3 preceding siblings ...)
2017-01-27 10:39 ` [PATCH v8 21/25] target-arm: don't generate WFE/YIELD calls for MTTCG Alex Bennée
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-27 10:39 ` [PATCH v8 23/25] target-arm: introduce ARM_CP_EXIT_PC Alex Bennée
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
open list:ARM
This is a purely mechanical change to make the ARM_CP flags neatly
align and use a consistent format so it is easier to see which bit
each flag is.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/cpu.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 274ef17562..f56a96c675 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1398,20 +1398,20 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
* need to be surrounded by gen_io_start()/gen_io_end(). In particular,
* registers which implement clocks or timers require this.
*/
-#define ARM_CP_SPECIAL 1
-#define ARM_CP_CONST 2
-#define ARM_CP_64BIT 4
-#define ARM_CP_SUPPRESS_TB_END 8
-#define ARM_CP_OVERRIDE 16
-#define ARM_CP_ALIAS 32
-#define ARM_CP_IO 64
-#define ARM_CP_NO_RAW 128
-#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
-#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
-#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8))
-#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 8))
-#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 8))
-#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA
+#define ARM_CP_SPECIAL (1 << 0)
+#define ARM_CP_CONST (1 << 1)
+#define ARM_CP_64BIT (1 << 2)
+#define ARM_CP_SUPPRESS_TB_END (1 << 3)
+#define ARM_CP_OVERRIDE (1 << 4)
+#define ARM_CP_ALIAS (1 << 5)
+#define ARM_CP_IO (1 << 6)
+#define ARM_CP_NO_RAW (1 << 7)
+#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
+#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
+#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8))
+#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 8))
+#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 8))
+#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA
/* Used only as a terminator for ARMCPRegInfo lists */
#define ARM_CP_SENTINEL 0xffff
/* Mask of only the flag bits in a type field */
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 23/25] target-arm: introduce ARM_CP_EXIT_PC
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
` (4 preceding siblings ...)
2017-01-27 10:39 ` [PATCH v8 22/25] target-arm/cpu.h: make ARM_CP defined consistent Alex Bennée
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-27 10:39 ` [PATCH v8 24/25] target-arm: ensure all cross vCPUs TLB flushes complete Alex Bennée
2017-01-27 10:39 ` [PATCH v8 25/25] tcg: enable MTTCG by default for ARM on x86 hosts Alex Bennée
7 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
open list:ARM
Some helpers may trigger an immediate exit of the cpu_loop. If this
happens the PC need to be rectified to ensure the restart will begin
on the next instruction.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/cpu.h | 3 ++-
target/arm/translate-a64.c | 4 ++++
target/arm/translate.c | 4 ++++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f56a96c675..1b0670ae11 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1411,7 +1411,8 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8))
#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 8))
#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 8))
-#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA
+#define ARM_CP_EXIT_PC (ARM_CP_SPECIAL | (6 << 8))
+#define ARM_LAST_SPECIAL ARM_CP_EXIT_PC
/* Used only as a terminator for ARMCPRegInfo lists */
#define ARM_CP_SENTINEL 0xffff
/* Mask of only the flag bits in a type field */
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 05162f335e..a3f37d8bec 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1561,6 +1561,10 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread,
tcg_rt = cpu_reg(s, rt);
gen_helper_dc_zva(cpu_env, tcg_rt);
return;
+ case ARM_CP_EXIT_PC:
+ /* The helper may exit the cpu_loop so ensure PC is correct */
+ gen_a64_set_pc_im(s->pc);
+ break;
default:
break;
}
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 444a24c2b6..7bd18cd25d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -7508,6 +7508,10 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn)
gen_set_pc_im(s, s->pc);
s->is_jmp = DISAS_WFI;
return 0;
+ case ARM_CP_EXIT_PC:
+ /* The helper may exit the cpu_loop so ensure PC is correct */
+ gen_set_pc_im(s, s->pc);
+ break;
default:
break;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 24/25] target-arm: ensure all cross vCPUs TLB flushes complete
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
` (5 preceding siblings ...)
2017-01-27 10:39 ` [PATCH v8 23/25] target-arm: introduce ARM_CP_EXIT_PC Alex Bennée
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-31 23:37 ` Richard Henderson
2017-01-27 10:39 ` [PATCH v8 25/25] tcg: enable MTTCG by default for ARM on x86 hosts Alex Bennée
7 siblings, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
open list:ARM
Previously flushes on other vCPUs would only get serviced when they
exited their TranslationBlocks. While this isn't overly problematic it
violates the semantics of TLB flush from the point of view of source
vCPU.
To solve this we call the cputlb *_all_cpus() functions to do the
flushes and ask it to ensure all flushes are completed before we start
the next instruction. As this involves exiting the cpu_loop we need to
ensure the PC is saved before the tlb helper functions are called.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v8
- fixup merge for bitmap based API
- fixup new _synced API
---
target/arm/helper.c | 194 +++++++++++++++++++++++-----------------------------
1 file changed, 84 insertions(+), 110 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0c6fb4add7..b3eb4d7430 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -536,41 +536,33 @@ static void tlbimvaa_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbiall_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush(other_cs);
- }
+ tlb_flush_all_cpus_synced(cs);
}
static void tlbiasid_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush(other_cs);
- }
+ tlb_flush_all_cpus_synced(cs);
}
static void tlbimva_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush_page(other_cs, value & TARGET_PAGE_MASK);
- }
+ tlb_flush_page_all_cpus_synced(cs, value & TARGET_PAGE_MASK);
}
static void tlbimvaa_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush_page(other_cs, value & TARGET_PAGE_MASK);
- }
+ tlb_flush_page_all_cpus_synced(cs, value & TARGET_PAGE_MASK);
}
static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -585,12 +577,10 @@ static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S12NSE1 |
ARMMMUBit_S12NSE0 | ARMMMUBit_S2NS);
- }
}
static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -617,7 +607,7 @@ static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
uint64_t pageaddr;
if (!arm_feature(env, ARM_FEATURE_EL2) || !(env->cp15.scr_el3 & SCR_NS)) {
@@ -626,9 +616,7 @@ static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 40);
- CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S2NS);
- }
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, ARMMMUBit_S2NS);
}
static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -642,11 +630,9 @@ static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbiall_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1E2);
- }
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S1E2);
}
static void tlbimva_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -661,12 +647,10 @@ static void tlbimva_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
uint64_t pageaddr = value & ~MAKE_64BIT_MASK(0, 12);
- CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1E2);
- }
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, ARMMMUBit_S1E2);
}
static const ARMCPRegInfo cp_reginfo[] = {
@@ -1335,14 +1319,16 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
static const ARMCPRegInfo v7mp_cp_reginfo[] = {
/* 32 bit TLB invalidates, Inner Shareable */
{ .name = "TLBIALLIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 0,
- .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiall_is_write },
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL1_W,
+ .writefn = tlbiall_is_write },
{ .name = "TLBIMVAIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 1,
- .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimva_is_write },
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL1_W,
+ .writefn = tlbimva_is_write },
{ .name = "TLBIASIDIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 2,
- .type = ARM_CP_NO_RAW, .access = PL1_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL1_W,
.writefn = tlbiasid_is_write },
{ .name = "TLBIMVAAIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 3,
- .type = ARM_CP_NO_RAW, .access = PL1_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL1_W,
.writefn = tlbimvaa_is_write },
REGINFO_SENTINEL
};
@@ -2856,8 +2842,7 @@ static CPAccessResult aa64_cacheop_access(CPUARMState *env,
static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- ARMCPU *cpu = arm_env_get_cpu(env);
- CPUState *cs = CPU(cpu);
+ CPUState *cs = ENV_GET_CPU(env);
if (arm_is_secure_below_el3(env)) {
tlb_flush_by_mmuidx(cs, ARMMMUBit_S1SE1 | ARMMMUBit_S1SE0);
@@ -2869,16 +2854,15 @@ static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbi_aa64_vmalle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
+ CPUState *cs = ENV_GET_CPU(env);
bool sec = arm_is_secure_below_el3(env);
- CPUState *other_cs;
- CPU_FOREACH(other_cs) {
- if (sec) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1SE1 | ARMMMUBit_S1SE0);
- } else {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
- ARMMMUBit_S12NSE0);
- }
+ if (sec) {
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S1SE1
+ | ARMMMUBit_S1SE0);
+ } else {
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S12NSE1
+ | ARMMMUBit_S12NSE0);
}
}
@@ -2931,39 +2915,34 @@ static void tlbi_aa64_alle1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
*/
bool sec = arm_is_secure_below_el3(env);
bool has_el2 = arm_feature(env, ARM_FEATURE_EL2);
- CPUState *other_cs;
-
- CPU_FOREACH(other_cs) {
- if (sec) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1SE1 | ARMMMUBit_S1SE0);
- } else if (has_el2) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
- ARMMMUBit_S12NSE0 | ARMMMUBit_S2NS);
- } else {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S12NSE1 |
- ARMMMUBit_S12NSE0);
- }
+ CPUState *cs = ENV_GET_CPU(env);
+
+ if (sec) {
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S1SE1
+ | ARMMMUBit_S1SE0);
+ } else if (has_el2) {
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S12NSE1
+ | ARMMMUBit_S12NSE0 | ARMMMUBit_S2NS);
+ } else {
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S12NSE1
+ | ARMMMUBit_S12NSE0);
}
}
static void tlbi_aa64_alle2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1E2);
- }
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S1E2);
}
static void tlbi_aa64_alle3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
- CPU_FOREACH(other_cs) {
- tlb_flush_by_mmuidx(other_cs, ARMMMUBit_S1E3);
- }
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUBit_S1E3);
}
static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3018,41 +2997,38 @@ static void tlbi_aa64_vae3_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
+ ARMCPU *cpu = arm_env_get_cpu(env);
+ CPUState *cs = CPU(cpu);
bool sec = arm_is_secure_below_el3(env);
- CPUState *other_cs;
uint64_t pageaddr = sextract64(value << 12, 0, 56);
- CPU_FOREACH(other_cs) {
- if (sec) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1SE1 |
- ARMMMUBit_S1SE0);
- } else {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S12NSE1 |
- ARMMMUBit_S12NSE0);
- }
+ if (sec) {
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
+ ARMMMUBit_S1SE1 |
+ ARMMMUBit_S1SE0);
+ } else {
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr,
+ ARMMMUBit_S12NSE1 |
+ ARMMMUBit_S12NSE0);
}
}
static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
- CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1E2);
- }
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, ARMMMUBit_S1E2);
}
static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
uint64_t pageaddr = sextract64(value << 12, 0, 56);
- CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S1E3);
- }
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, ARMMMUBit_S1E3);
}
static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3080,7 +3056,7 @@ static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
- CPUState *other_cs;
+ CPUState *cs = ENV_GET_CPU(env);
uint64_t pageaddr;
if (!arm_feature(env, ARM_FEATURE_EL2) || !(env->cp15.scr_el3 & SCR_NS)) {
@@ -3089,9 +3065,7 @@ static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
pageaddr = sextract64(value << 12, 0, 48);
- CPU_FOREACH(other_cs) {
- tlb_flush_page_by_mmuidx(other_cs, pageaddr, ARMMMUBit_S2NS);
- }
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, ARMMMUBit_S2NS);
}
static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3248,27 +3222,27 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
/* TLBI operations */
{ .name = "TLBI_VMALLE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 0,
- .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vmalle1is_write },
{ .name = "TLBI_VAE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 1,
- .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vae1is_write },
{ .name = "TLBI_ASIDE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 2,
- .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vmalle1is_write },
{ .name = "TLBI_VAAE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 3,
- .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vae1is_write },
{ .name = "TLBI_VALE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 5,
- .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vae1is_write },
{ .name = "TLBI_VAALE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 7,
- .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vae1is_write },
{ .name = "TLBI_VMALLE1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 0,
@@ -3296,19 +3270,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbi_aa64_vae1_write },
{ .name = "TLBI_IPAS2E1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1,
- .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_ipas2e1is_write },
{ .name = "TLBI_IPAS2LE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5,
- .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_ipas2e1is_write },
{ .name = "TLBI_ALLE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 4,
- .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_alle1is_write },
{ .name = "TLBI_VMALLS12E1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 6,
- .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_alle1is_write },
{ .name = "TLBI_IPAS2E1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1,
@@ -3324,7 +3298,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbi_aa64_alle1_write },
{ .name = "TLBI_VMALLS12E1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 7, .opc2 = 6,
- .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_alle1is_write },
#ifndef CONFIG_USER_ONLY
/* 64 bit address translation operations */
@@ -3370,7 +3344,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
{ .name = "TLBIMVALIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 5,
.type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimva_is_write },
{ .name = "TLBIMVAALIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 7,
- .type = ARM_CP_NO_RAW, .access = PL1_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL1_W,
.writefn = tlbimvaa_is_write },
{ .name = "TLBIMVAL", .cp = 15, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 5,
.type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimva_write },
@@ -3381,7 +3355,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbimva_hyp_write },
{ .name = "TLBIMVALHIS",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 5,
- .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL2_W,
.writefn = tlbimva_hyp_is_write },
{ .name = "TLBIIPAS2",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1,
@@ -3389,7 +3363,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbiipas2_write },
{ .name = "TLBIIPAS2IS",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1,
- .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL2_W,
.writefn = tlbiipas2_is_write },
{ .name = "TLBIIPAS2L",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 5,
@@ -3397,7 +3371,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbiipas2_write },
{ .name = "TLBIIPAS2LIS",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5,
- .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL2_W,
.writefn = tlbiipas2_is_write },
/* 32 bit cache operations */
{ .name = "ICIALLUIS", .cp = 15, .opc1 = 0, .crn = 7, .crm = 1, .opc2 = 0,
@@ -3737,7 +3711,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
.writefn = tlbiall_nsnh_write },
{ .name = "TLBIALLNSNHIS",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 4,
- .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL2_W,
.writefn = tlbiall_nsnh_is_write },
{ .name = "TLBIALLH", .cp = 15, .opc1 = 4, .crn = 8, .crm = 7, .opc2 = 0,
.type = ARM_CP_NO_RAW, .access = PL2_W,
@@ -3749,7 +3723,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
.type = ARM_CP_NO_RAW, .access = PL2_W,
.writefn = tlbimva_hyp_write },
{ .name = "TLBIMVAHIS", .cp = 15, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 1,
- .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL2_W,
.writefn = tlbimva_hyp_is_write },
{ .name = "TLBI_ALLE2", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 7, .opc2 = 0,
@@ -3765,15 +3739,15 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
.writefn = tlbi_aa64_vae2_write },
{ .name = "TLBI_ALLE2IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 0,
- .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_alle2is_write },
{ .name = "TLBI_VAE2IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 1,
- .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC, .access = PL2_W,
.writefn = tlbi_aa64_vae2is_write },
{ .name = "TLBI_VALE2IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 5,
- .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vae2is_write },
#ifndef CONFIG_USER_ONLY
/* Unlike the other EL2-related AT operations, these must
@@ -3960,15 +3934,15 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
.resetvalue = 0 },
{ .name = "TLBI_ALLE3IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 6, .crn = 8, .crm = 3, .opc2 = 0,
- .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_alle3is_write },
{ .name = "TLBI_VAE3IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 6, .crn = 8, .crm = 3, .opc2 = 1,
- .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vae3is_write },
{ .name = "TLBI_VALE3IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 6, .crn = 8, .crm = 3, .opc2 = 5,
- .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_EXIT_PC,
.writefn = tlbi_aa64_vae3is_write },
{ .name = "TLBI_ALLE3", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 6, .crn = 8, .crm = 7, .opc2 = 0,
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 25/25] tcg: enable MTTCG by default for ARM on x86 hosts
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
` (6 preceding siblings ...)
2017-01-27 10:39 ` [PATCH v8 24/25] target-arm: ensure all cross vCPUs TLB flushes complete Alex Bennée
@ 2017-01-27 10:39 ` Alex Bennée
2017-01-31 23:39 ` Richard Henderson
7 siblings, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2017-01-27 10:39 UTC (permalink / raw)
To: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Alex Bennée,
open list:ARM
This enables the multi-threaded system emulation by default for ARMv7
and ARMv8 guests using the x86_64 TCG backend. This is because on the
guest side:
- The ARM translate.c/translate-64.c have been converted to
- use MTTCG safe atomic primitives
- emit the appropriate barrier ops
- The ARM machine has been updated to
- hold the BQL when modifying shared cross-vCPU state
- defer cpu_reset to async safe work
All the host backends support the barrier and atomic primitives but
need to provide same-or-better support for normal load/store
operations.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v7
- drop configure check for backend
- declare backend memory order for x86
- declare guest memory order for ARM
- add configure snippet to set TARGET_SUPPORTS_MTTCG
---
configure | 6 ++++++
target/arm/cpu.h | 3 +++
tcg/i386/tcg-target.h | 16 ++++++++++++++++
3 files changed, 25 insertions(+)
diff --git a/configure b/configure
index 86fd833feb..9f2a665f5b 100755
--- a/configure
+++ b/configure
@@ -5879,6 +5879,7 @@ mkdir -p $target_dir
echo "# Automatically generated by configure - do not modify" > $config_target_mak
bflt="no"
+mttcg="no"
interp_prefix1=$(echo "$interp_prefix" | sed "s/%M/$target_name/g")
gdb_xml_files=""
@@ -5897,11 +5898,13 @@ case "$target_name" in
arm|armeb)
TARGET_ARCH=arm
bflt="yes"
+ mttcg="yes"
gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
;;
aarch64)
TARGET_BASE_ARCH=arm
bflt="yes"
+ mttcg="yes"
gdb_xml_files="aarch64-core.xml aarch64-fpu.xml arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
;;
cris)
@@ -6066,6 +6069,9 @@ if test "$target_bigendian" = "yes" ; then
fi
if test "$target_softmmu" = "yes" ; then
echo "CONFIG_SOFTMMU=y" >> $config_target_mak
+ if test "$mttcg" = "yes" ; then
+ echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
+ fi
fi
if test "$target_user_only" = "yes" ; then
echo "CONFIG_USER_ONLY=y" >> $config_target_mak
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1b0670ae11..47a42ec6d6 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -29,6 +29,9 @@
# define TARGET_LONG_BITS 32
#endif
+/* ARM processors have a weak memory model */
+#define TCG_DEFAULT_MO (0)
+
#define CPUArchState struct CPUARMState
#include "qemu-common.h"
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index 21d96ec35c..536190f647 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -165,4 +165,20 @@ static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
{
}
+/* This defines the natural memory order supported by this
+ * architecture before guarantees made by various barrier
+ * instructions.
+ *
+ * The x86 has a pretty strong memory ordering which only really
+ * allows for some stores to be re-ordered after loads.
+ */
+#include "tcg-mo.h"
+
+static inline int get_tcg_target_mo(void)
+{
+ return TCG_MO_ALL & ~TCG_MO_LD_ST;
+}
+
+#define TCG_TARGET_DEFAULT_MO get_tcg_target_mo()
+
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v8 08/25] tcg: drop global lock during TCG code execution
2017-01-27 10:39 ` [PATCH v8 08/25] tcg: drop global lock during TCG code execution Alex Bennée
@ 2017-01-29 21:33 ` Pranith Kumar
2017-01-30 9:57 ` Alex Bennée
0 siblings, 1 reply; 19+ messages in thread
From: Pranith Kumar @ 2017-01-29 21:33 UTC (permalink / raw)
To: Alex Bennée
Cc: mttcg, qemu-devel, fred.konrad, a.rigo, cota, nikunj, mark.burton,
pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Peter Crosthwaite,
Michael S. Tsirkin, Eduardo Habkost, David Gibson, Alexander Graf,
open list:ARM cores, open list:PowerPC
Alex Bennée writes:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> This finally allows TCG to benefit from the iothread introduction: Drop
> the global mutex while running pure TCG CPU code. Reacquire the lock
> when entering MMIO or PIO emulation, or when leaving the TCG loop.
>
> We have to revert a few optimization for the current TCG threading
> model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not
> kicking it in qemu_cpu_kick. We also need to disable RAM block
> reordering until we have a more efficient locking mechanism at hand.
>
> Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.
> These numbers demonstrate where we gain something:
>
> 20338 jan 20 0 331m 75m 6904 R 99 0.9 0:50.95 qemu-system-arm
> 20337 jan 20 0 331m 75m 6904 S 20 0.9 0:26.50 qemu-system-arm
>
> The guest CPU was fully loaded, but the iothread could still run mostly
> independent on a second core. Without the patch we don't get beyond
>
> 32206 jan 20 0 330m 73m 7036 R 82 0.9 1:06.00 qemu-system-arm
> 32204 jan 20 0 330m 73m 7036 S 21 0.9 0:17.03 qemu-system-arm
>
> We don't benefit significantly, though, when the guest is not fully
> loading a host CPU.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>
> [FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> [EGC: fixed iothread lock for cpu-exec IRQ handling]
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> [AJB: -smp single-threaded fix, clean commit msg, BQL fixes]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
>
> ---
> v8:
> - merged in BQL fixes for PPC target: ppc_set_irq
> - merged in BQL fixes for ARM target: ARM_CP_IO helpers
> - merged in BQL fixes for ARM target: arm_call_el_change_hook
>
> v5 (ajb, base patches):
> - added an assert to BQL unlock/lock functions instead of hanging
> - ensure all cpu->interrupt_requests *modifications* protected by BQL
> - add a re-read on cpu->interrupt_request for correctness
> - BQL fixes for:
> - assert BQL held for PPC hypercalls (emulate_spar_hypercall)
> - SCLP service calls on s390x
> - merge conflict with kick timer patch
> v4 (ajb, base patches):
> - protect cpu->interrupt updates with BQL
> - fix wording io_mem_notdirty calls
> - s/we/with/
> v3 (ajb, base-patches):
> - stale iothread_unlocks removed (cpu_exit/resume_from_signal deals
> with it in the longjmp).
> - fix re-base conflicts
> v2 (ajb):
> - merge with tcg: grab iothread lock in cpu-exec interrupt handling
> - use existing fns for tracking lock state
> - lock iothread for mem_region
> - add assert on mem region modification
> - ensure smm_helper holds iothread
> - Add JK s-o-b
> - Fix-up FK s-o-b annotation
> v1 (ajb, base-patches):
> - SMP failure now fixed by previous commit
>
> Changes from Fred Konrad (mttcg-v7 via paolo):
> * Rebase on the current HEAD.
> * Fixes a deadlock in qemu_devices_reset().
> * Remove the mutex in address_space_*
> ---
> cpu-exec.c | 20 ++++++++++++++++++--
> cpus.c | 28 +++++-----------------------
> cputlb.c | 21 ++++++++++++++++++++-
> exec.c | 12 +++++++++---
> hw/core/irq.c | 1 +
> hw/i386/kvmvapic.c | 4 ++--
> hw/intc/arm_gicv3_cpuif.c | 3 +++
> hw/ppc/ppc.c | 16 +++++++++++++++-
> hw/ppc/spapr.c | 3 +++
> include/qom/cpu.h | 1 +
> memory.c | 2 ++
> qom/cpu.c | 10 ++++++++++
> target/arm/helper.c | 6 ++++++
> target/arm/op_helper.c | 43 +++++++++++++++++++++++++++++++++++++++----
> target/i386/smm_helper.c | 7 +++++++
> target/s390x/misc_helper.c | 5 ++++-
> translate-all.c | 9 +++++++--
> translate-common.c | 21 +++++++++++----------
> 18 files changed, 163 insertions(+), 49 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index f9e836c8dd..f42a128bdf 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -29,6 +29,7 @@
> #include "qemu/rcu.h"
> #include "exec/tb-hash.h"
> #include "exec/log.h"
> +#include "qemu/main-loop.h"
> #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
> #include "hw/i386/apic.h"
> #endif
> @@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)
> if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
> && replay_interrupt()) {
> X86CPU *x86_cpu = X86_CPU(cpu);
> + qemu_mutex_lock_iothread();
> apic_poll_irq(x86_cpu->apic_state);
> cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
> + qemu_mutex_unlock_iothread();
> }
> #endif
> if (!cpu_has_work(cpu)) {
> @@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
> #else
> if (replay_exception()) {
> CPUClass *cc = CPU_GET_CLASS(cpu);
> + qemu_mutex_lock_iothread();
> cc->do_interrupt(cpu);
> + qemu_mutex_unlock_iothread();
> cpu->exception_index = -1;
> } else if (!replay_has_interrupt()) {
> /* give a chance to iothread in replay mode */
> @@ -469,9 +474,11 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
> TranslationBlock **last_tb)
> {
> CPUClass *cc = CPU_GET_CLASS(cpu);
> - int interrupt_request = cpu->interrupt_request;
>
> - if (unlikely(interrupt_request)) {
> + if (unlikely(atomic_read(&cpu->interrupt_request))) {
> + int interrupt_request;
> + qemu_mutex_lock_iothread();
> + interrupt_request = cpu->interrupt_request;
> if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
> /* Mask out external interrupts for this step. */
> interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
> @@ -526,7 +533,12 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
> the program flow was changed */
> *last_tb = NULL;
> }
> +
> + /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */
> + qemu_mutex_unlock_iothread();
> }
> +
> +
> if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {
> atomic_set(&cpu->exit_request, 0);
> cpu->exception_index = EXCP_INTERRUPT;
> @@ -656,8 +668,12 @@ int cpu_exec(CPUState *cpu)
> g_assert(cpu == current_cpu);
> g_assert(cc == CPU_GET_CLASS(cpu));
> #endif /* buggy compiler */
> +
> cpu->can_do_io = 1;
> tb_lock_reset();
> + if (qemu_mutex_iothread_locked()) {
> + qemu_mutex_unlock_iothread();
> + }
> }
> } /* for(;;) */
>
> diff --git a/cpus.c b/cpus.c
> index 6d64199831..c48bc8d5b3 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1026,8 +1026,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)
> #endif /* _WIN32 */
>
> static QemuMutex qemu_global_mutex;
> -static QemuCond qemu_io_proceeded_cond;
> -static unsigned iothread_requesting_mutex;
>
> static QemuThread io_thread;
>
> @@ -1041,7 +1039,6 @@ void qemu_init_cpu_loop(void)
> qemu_init_sigbus();
> qemu_cond_init(&qemu_cpu_cond);
> qemu_cond_init(&qemu_pause_cond);
> - qemu_cond_init(&qemu_io_proceeded_cond);
> qemu_mutex_init(&qemu_global_mutex);
>
> qemu_thread_get_self(&io_thread);
> @@ -1084,10 +1081,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
>
> start_tcg_kick_timer();
>
> - while (iothread_requesting_mutex) {
> - qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);
> - }
> -
> CPU_FOREACH(cpu) {
> qemu_wait_io_event_common(cpu);
> }
> @@ -1248,9 +1241,11 @@ static int tcg_cpu_exec(CPUState *cpu)
> cpu->icount_decr.u16.low = decr;
> cpu->icount_extra = count;
> }
> + qemu_mutex_unlock_iothread();
> cpu_exec_start(cpu);
> ret = cpu_exec(cpu);
> cpu_exec_end(cpu);
> + qemu_mutex_lock_iothread();
> #ifdef CONFIG_PROFILER
> tcg_time += profile_getclock() - ti;
> #endif
> @@ -1478,27 +1473,14 @@ bool qemu_mutex_iothread_locked(void)
>
> void qemu_mutex_lock_iothread(void)
> {
> - atomic_inc(&iothread_requesting_mutex);
> - /* In the simple case there is no need to bump the VCPU thread out of
> - * TCG code execution.
> - */
> - if (!tcg_enabled() || qemu_in_vcpu_thread() ||
> - !first_cpu || !first_cpu->created) {
> - qemu_mutex_lock(&qemu_global_mutex);
> - atomic_dec(&iothread_requesting_mutex);
> - } else {
> - if (qemu_mutex_trylock(&qemu_global_mutex)) {
> - qemu_cpu_kick_rr_cpu();
> - qemu_mutex_lock(&qemu_global_mutex);
> - }
> - atomic_dec(&iothread_requesting_mutex);
> - qemu_cond_broadcast(&qemu_io_proceeded_cond);
> - }
> + g_assert(!qemu_mutex_iothread_locked());
How about using tcg_debug_assert here...
> + qemu_mutex_lock(&qemu_global_mutex);
> iothread_locked = true;
> }
>
> void qemu_mutex_unlock_iothread(void)
> {
> + g_assert(qemu_mutex_iothread_locked());
... and here?
> iothread_locked = false;
> qemu_mutex_unlock(&qemu_global_mutex);
> }
> diff --git a/cputlb.c b/cputlb.c
> index 6c39927455..1cc9d9da51 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -18,6 +18,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> #include "cpu.h"
> #include "exec/exec-all.h"
> #include "exec/memory.h"
> @@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
> hwaddr physaddr = iotlbentry->addr;
> MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
> uint64_t val;
> + bool locked = false;
>
> physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
> cpu->mem_io_pc = retaddr;
> @@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
> }
>
> cpu->mem_io_vaddr = addr;
> +
> + if (mr->global_locking) {
> + qemu_mutex_lock_iothread();
> + locked = true;
> + }
> memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);
> + if (locked) {
> + qemu_mutex_unlock_iothread();
> + }
> +
> return val;
> }
>
> @@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
> CPUState *cpu = ENV_GET_CPU(env);
> hwaddr physaddr = iotlbentry->addr;
> MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
> + bool locked = false;
>
> physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
> if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {
> cpu_io_recompile(cpu, retaddr);
> }
> -
I am not really opposed but, I like having this line.. not sure why you are removing it? :)
> cpu->mem_io_vaddr = addr;
> cpu->mem_io_pc = retaddr;
> +
> + if (mr->global_locking) {
> + qemu_mutex_lock_iothread();
> + locked = true;
> + }
> memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);
> + if (locked) {
> + qemu_mutex_unlock_iothread();
> + }
> }
>
> /* Return true if ADDR is present in the victim tlb, and has been copied
> diff --git a/exec.c b/exec.c
> index f2bed92b64..87cf0db91e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2133,9 +2133,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
> }
> cpu->watchpoint_hit = wp;
>
> - /* The tb_lock will be reset when cpu_loop_exit or
> - * cpu_loop_exit_noexc longjmp back into the cpu_exec
> - * main loop.
> + /* Both tb_lock and iothread_mutex will be reset when
> + * cpu_loop_exit or cpu_loop_exit_noexc longjmp
> + * back into the cpu_exec main loop.
> */
> tb_lock();
> tb_check_watchpoint(cpu);
> @@ -2370,8 +2370,14 @@ static void io_mem_init(void)
> memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);
> memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
> NULL, UINT64_MAX);
> +
> + /* io_mem_notdirty calls tb_invalidate_phys_page_fast,
> + * which can be called without the iothread mutex.
> + */
> memory_region_init_io(&io_mem_notdirty, NULL, ¬dirty_mem_ops, NULL,
> NULL, UINT64_MAX);
> + memory_region_clear_global_locking(&io_mem_notdirty);
> +
> memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,
> NULL, UINT64_MAX);
> }
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 49ff2e64fe..b98d1d69f5 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -22,6 +22,7 @@
> * THE SOFTWARE.
> */
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> #include "qemu-common.h"
> #include "hw/irq.h"
> #include "qom/object.h"
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index 702e281dc8..b3c49b2c61 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -451,8 +451,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
> resume_all_vcpus();
>
> if (!kvm_enabled()) {
> - /* tb_lock will be reset when cpu_loop_exit_noexc longjmps
> - * back into the cpu_exec loop. */
> + /* Both tb_lock and iothread_mutex will be reset when
> + * longjmps back into the cpu_exec loop. */
missing cpu_loop_exit_noexc?
> tb_lock();
> tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
> cpu_loop_exit_noexc(cs);
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index a9ee7fddf9..2624d8d909 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -14,6 +14,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/bitops.h"
> +#include "qemu/main-loop.h"
> #include "trace.h"
> #include "gicv3_internal.h"
> #include "cpu.h"
> @@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
> ARMCPU *cpu = ARM_CPU(cs->cpu);
> CPUARMState *env = &cpu->env;
>
> + g_assert(qemu_mutex_iothread_locked());
> +
tcg_debug_assert()?
> trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,
> cs->hppi.grp, cs->hppi.prio);
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 8945869009..59c3faa6c8 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> - unsigned int old_pending = env->pending_interrupts;
> + unsigned int old_pending;
> + bool locked = false;
> +
> + /* We may already have the BQL if coming from the reset path */
> + if (!qemu_mutex_iothread_locked()) {
> + locked = true;
> + qemu_mutex_lock_iothread();
> + }
> +
> + old_pending = env->pending_interrupts;
>
> if (level) {
> env->pending_interrupts |= 1 << n_IRQ;
> @@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
> #endif
> }
>
> +
> LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
> "req %08x\n", __func__, env, n_IRQ, level,
> env->pending_interrupts, CPU(cpu)->interrupt_request);
> +
> + if (locked) {
> + qemu_mutex_unlock_iothread();
> + }
> }
>
> /* PowerPC 6xx / 7xx internal IRQ controller */
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a642e663d4..745743d64b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1012,6 +1012,9 @@ static void emulate_spapr_hypercall(PowerPCCPU *cpu)
> {
> CPUPPCState *env = &cpu->env;
>
> + /* The TCG path should also be holding the BQL at this point */
> + g_assert(qemu_mutex_iothread_locked());
> +
tcg_debug_assert()?
> if (msr_pr) {
> hcall_dprintf("Hypercall made with MSR[PR]=1\n");
> env->gpr[3] = H_PRIVILEGE;
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 11db2015a4..1a06ae5938 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -325,6 +325,7 @@ struct CPUState {
> bool unplug;
> bool crash_occurred;
> bool exit_request;
> + /* updates protected by BQL */
> uint32_t interrupt_request;
> int singlestep_enabled;
> int64_t icount_extra;
> diff --git a/memory.c b/memory.c
> index 2bfc37f65c..7d7b285e41 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)
> AddressSpace *as;
>
> assert(memory_region_transaction_depth);
> + assert(qemu_mutex_iothread_locked());
> +
> --memory_region_transaction_depth;
> if (!memory_region_transaction_depth) {
> if (memory_region_update_pending) {
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 7f575879f6..bd77c05cd0 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
> error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
> }
>
> +/* Resetting the IRQ comes from across the code base so we take the
> + * BQL here if we need to. cpu_interrupt assumes it is held.*/
> void cpu_reset_interrupt(CPUState *cpu, int mask)
> {
> + bool need_lock = !qemu_mutex_iothread_locked();
> +
> + if (need_lock) {
> + qemu_mutex_lock_iothread();
> + }
> cpu->interrupt_request &= ~mask;
> + if (need_lock) {
> + qemu_mutex_unlock_iothread();
> + }
> }
>
> void cpu_exit(CPUState *cpu)
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 7111c8cf18..84d789be93 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6693,6 +6693,12 @@ void arm_cpu_do_interrupt(CPUState *cs)
> arm_cpu_do_interrupt_aarch32(cs);
> }
>
> + /* Hooks may change global state so BQL should be held, also the
> + * BQL needs to be held for any modification of
> + * cs->interrupt_request.
> + */
> + g_assert(qemu_mutex_iothread_locked());
> +
> arm_call_el_change_hook(cpu);
>
> if (!kvm_enabled()) {
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index ba796d898e..e1a883c595 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -18,6 +18,7 @@
> */
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> +#include "qemu/main-loop.h"
> #include "cpu.h"
> #include "exec/helper-proto.h"
> #include "internals.h"
> @@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)
> */
> env->regs[15] &= (env->thumb ? ~1 : ~3);
>
> + qemu_mutex_lock_iothread();
> arm_call_el_change_hook(arm_env_get_cpu(env));
> + qemu_mutex_unlock_iothread();
> }
>
> /* Access to user mode registers from privileged modes. */
> @@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
> {
> const ARMCPRegInfo *ri = rip;
>
> - ri->writefn(env, ri, value);
> + if (ri->type & ARM_CP_IO) {
> + qemu_mutex_lock_iothread();
> + ri->writefn(env, ri, value);
> + qemu_mutex_unlock_iothread();
> + } else {
> + ri->writefn(env, ri, value);
> + }
> }
>
> uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)
> {
> const ARMCPRegInfo *ri = rip;
> + uint32_t res;
>
> - return ri->readfn(env, ri);
> + if (ri->type & ARM_CP_IO) {
> + qemu_mutex_lock_iothread();
> + res = ri->readfn(env, ri);
> + qemu_mutex_unlock_iothread();
> + } else {
> + res = ri->readfn(env, ri);
> + }
> +
> + return res;
> }
>
> void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)
> {
> const ARMCPRegInfo *ri = rip;
>
> - ri->writefn(env, ri, value);
> + if (ri->type & ARM_CP_IO) {
> + qemu_mutex_lock_iothread();
> + ri->writefn(env, ri, value);
> + qemu_mutex_unlock_iothread();
> + } else {
> + ri->writefn(env, ri, value);
> + }
> }
>
> uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)
> {
> const ARMCPRegInfo *ri = rip;
> + uint64_t res;
> +
> + if (ri->type & ARM_CP_IO) {
> + qemu_mutex_lock_iothread();
> + res = ri->readfn(env, ri);
> + qemu_mutex_unlock_iothread();
> + } else {
> + res = ri->readfn(env, ri);
> + }
>
> - return ri->readfn(env, ri);
> + return res;
> }
>
> void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
> @@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)
> cur_el, new_el, env->pc);
> }
>
> + qemu_mutex_lock_iothread();
> arm_call_el_change_hook(arm_env_get_cpu(env));
> + qemu_mutex_unlock_iothread();
>
> return;
>
> diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c
> index 4dd6a2c544..f051a77c4a 100644
> --- a/target/i386/smm_helper.c
> +++ b/target/i386/smm_helper.c
> @@ -18,6 +18,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> #include "cpu.h"
> #include "exec/helper-proto.h"
> #include "exec/log.h"
> @@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)
> #define SMM_REVISION_ID 0x00020000
> #endif
>
> +/* Called with iothread lock taken */
> void cpu_smm_update(X86CPU *cpu)
> {
> CPUX86State *env = &cpu->env;
> bool smm_enabled = (env->hflags & HF_SMM_MASK);
>
> + g_assert(qemu_mutex_iothread_locked());
> +
> if (cpu->smram) {
> memory_region_set_enabled(cpu->smram, smm_enabled);
> }
> @@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)
> }
> env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;
> env->hflags &= ~HF_SMM_MASK;
> +
> + qemu_mutex_lock_iothread();
> cpu_smm_update(cpu);
> + qemu_mutex_unlock_iothread();
>
> qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
> log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index c9604ea9c7..3cb942e8bb 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -25,6 +25,7 @@
> #include "exec/helper-proto.h"
> #include "sysemu/kvm.h"
> #include "qemu/timer.h"
> +#include "qemu/main-loop.h"
> #include "exec/address-spaces.h"
> #ifdef CONFIG_KVM
> #include <linux/kvm.h>
> @@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
> /* SCLP service call */
> uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
> {
> + qemu_mutex_lock_iothread();
> int r = sclp_service_call(env, r1, r2);
> if (r < 0) {
> program_interrupt(env, -r, 4);
> - return 0;
> + r = 0;
> }
> + qemu_mutex_unlock_iothread();
> return r;
> }
>
> diff --git a/translate-all.c b/translate-all.c
> index 055436a676..41b36f04c6 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -55,6 +55,7 @@
> #include "translate-all.h"
> #include "qemu/bitmap.h"
> #include "qemu/timer.h"
> +#include "qemu/main-loop.h"
> #include "exec/log.h"
>
> /* #define DEBUG_TB_INVALIDATE */
> @@ -1521,7 +1522,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> #ifdef CONFIG_SOFTMMU
> /* len must be <= 8 and start must be a multiple of len.
> * Called via softmmu_template.h when code areas are written to with
> - * tb_lock held.
> + * iothread mutex not held.
> */
> void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
> {
Shouldn't this be both tb_lock and iothread mutex?
> @@ -1723,7 +1724,10 @@ void tb_check_watchpoint(CPUState *cpu)
>
> #ifndef CONFIG_USER_ONLY
> /* in deterministic execution mode, instructions doing device I/Os
> - must be at the end of the TB */
> + * must be at the end of the TB.
> + *
> + * Called by softmmu_template.h, with iothread mutex not held.
> + */
> void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
> {
> #if defined(TARGET_MIPS) || defined(TARGET_SH4)
> @@ -1935,6 +1939,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
>
> void cpu_interrupt(CPUState *cpu, int mask)
> {
> + g_assert(qemu_mutex_iothread_locked());
> cpu->interrupt_request |= mask;
> cpu->tcg_exit_req = 1;
> }
> diff --git a/translate-common.c b/translate-common.c
> index 5e989cdf70..d504dd0d33 100644
> --- a/translate-common.c
> +++ b/translate-common.c
> @@ -21,6 +21,7 @@
> #include "qemu-common.h"
> #include "qom/cpu.h"
> #include "sysemu/cpus.h"
> +#include "qemu/main-loop.h"
>
> uintptr_t qemu_real_host_page_size;
> intptr_t qemu_real_host_page_mask;
> @@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;
> static void tcg_handle_interrupt(CPUState *cpu, int mask)
> {
> int old_mask;
> + g_assert(qemu_mutex_iothread_locked());
>
> old_mask = cpu->interrupt_request;
> cpu->interrupt_request |= mask;
> @@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)
> */
> if (!qemu_cpu_is_self(cpu)) {
> qemu_cpu_kick(cpu);
> - return;
> - }
> -
> - if (use_icount) {
> - cpu->icount_decr.u16.high = 0xffff;
> - if (!cpu->can_do_io
> - && (mask & ~old_mask) != 0) {
> - cpu_abort(cpu, "Raised interrupt while not in I/O function");
> - }
> } else {
> - cpu->tcg_exit_req = 1;
> + if (use_icount) {
> + cpu->icount_decr.u16.high = 0xffff;
> + if (!cpu->can_do_io
> + && (mask & ~old_mask) != 0) {
> + cpu_abort(cpu, "Raised interrupt while not in I/O function");
> + }
> + } else {
> + cpu->tcg_exit_req = 1;
> + }
> }
> }
Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>
--
Pranith
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 08/25] tcg: drop global lock during TCG code execution
2017-01-29 21:33 ` Pranith Kumar
@ 2017-01-30 9:57 ` Alex Bennée
2017-01-30 16:52 ` Richard Henderson
0 siblings, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2017-01-30 9:57 UTC (permalink / raw)
To: Pranith Kumar
Cc: mttcg, qemu-devel, fred.konrad, a.rigo, cota, nikunj, mark.burton,
pbonzini, jan.kiszka, serge.fdrv, rth, peter.maydell,
claudio.fontana, bamvor.zhangjian, Peter Crosthwaite,
Michael S. Tsirkin, Eduardo Habkost, David Gibson, Alexander Graf,
open list:ARM cores, open list:PowerPC
Pranith Kumar <bobby.prani@gmail.com> writes:
> Alex Bennée writes:
>
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This finally allows TCG to benefit from the iothread introduction: Drop
>> the global mutex while running pure TCG CPU code. Reacquire the lock
>> when entering MMIO or PIO emulation, or when leaving the TCG loop.
>>
>> We have to revert a few optimization for the current TCG threading
>> model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not
>> kicking it in qemu_cpu_kick. We also need to disable RAM block
>> reordering until we have a more efficient locking mechanism at hand.
>>
>> Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.
>> These numbers demonstrate where we gain something:
>>
>> 20338 jan 20 0 331m 75m 6904 R 99 0.9 0:50.95 qemu-system-arm
>> 20337 jan 20 0 331m 75m 6904 S 20 0.9 0:26.50 qemu-system-arm
>>
>> The guest CPU was fully loaded, but the iothread could still run mostly
>> independent on a second core. Without the patch we don't get beyond
>>
>> 32206 jan 20 0 330m 73m 7036 R 82 0.9 1:06.00 qemu-system-arm
>> 32204 jan 20 0 330m 73m 7036 S 21 0.9 0:17.03 qemu-system-arm
>>
>> We don't benefit significantly, though, when the guest is not fully
>> loading a host CPU.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> Message-Id: <1439220437-23957-10-git-send-email-fred.konrad@greensocs.com>
>> [FK: Rebase, fix qemu_devices_reset deadlock, rm address_space_* mutex]
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> [EGC: fixed iothread lock for cpu-exec IRQ handling]
>> Signed-off-by: Emilio G. Cota <cota@braap.org>
>> [AJB: -smp single-threaded fix, clean commit msg, BQL fixes]
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>>
>> ---
>> v8:
>> - merged in BQL fixes for PPC target: ppc_set_irq
>> - merged in BQL fixes for ARM target: ARM_CP_IO helpers
>> - merged in BQL fixes for ARM target: arm_call_el_change_hook
>>
>> v5 (ajb, base patches):
>> - added an assert to BQL unlock/lock functions instead of hanging
>> - ensure all cpu->interrupt_requests *modifications* protected by BQL
>> - add a re-read on cpu->interrupt_request for correctness
>> - BQL fixes for:
>> - assert BQL held for PPC hypercalls (emulate_spar_hypercall)
>> - SCLP service calls on s390x
>> - merge conflict with kick timer patch
>> v4 (ajb, base patches):
>> - protect cpu->interrupt updates with BQL
>> - fix wording io_mem_notdirty calls
>> - s/we/with/
>> v3 (ajb, base-patches):
>> - stale iothread_unlocks removed (cpu_exit/resume_from_signal deals
>> with it in the longjmp).
>> - fix re-base conflicts
>> v2 (ajb):
>> - merge with tcg: grab iothread lock in cpu-exec interrupt handling
>> - use existing fns for tracking lock state
>> - lock iothread for mem_region
>> - add assert on mem region modification
>> - ensure smm_helper holds iothread
>> - Add JK s-o-b
>> - Fix-up FK s-o-b annotation
>> v1 (ajb, base-patches):
>> - SMP failure now fixed by previous commit
>>
>> Changes from Fred Konrad (mttcg-v7 via paolo):
>> * Rebase on the current HEAD.
>> * Fixes a deadlock in qemu_devices_reset().
>> * Remove the mutex in address_space_*
>> ---
>> cpu-exec.c | 20 ++++++++++++++++++--
>> cpus.c | 28 +++++-----------------------
>> cputlb.c | 21 ++++++++++++++++++++-
>> exec.c | 12 +++++++++---
>> hw/core/irq.c | 1 +
>> hw/i386/kvmvapic.c | 4 ++--
>> hw/intc/arm_gicv3_cpuif.c | 3 +++
>> hw/ppc/ppc.c | 16 +++++++++++++++-
>> hw/ppc/spapr.c | 3 +++
>> include/qom/cpu.h | 1 +
>> memory.c | 2 ++
>> qom/cpu.c | 10 ++++++++++
>> target/arm/helper.c | 6 ++++++
>> target/arm/op_helper.c | 43 +++++++++++++++++++++++++++++++++++++++----
>> target/i386/smm_helper.c | 7 +++++++
>> target/s390x/misc_helper.c | 5 ++++-
>> translate-all.c | 9 +++++++--
>> translate-common.c | 21 +++++++++++----------
>> 18 files changed, 163 insertions(+), 49 deletions(-)
>>
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index f9e836c8dd..f42a128bdf 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -29,6 +29,7 @@
>> #include "qemu/rcu.h"
>> #include "exec/tb-hash.h"
>> #include "exec/log.h"
>> +#include "qemu/main-loop.h"
>> #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
>> #include "hw/i386/apic.h"
>> #endif
>> @@ -388,8 +389,10 @@ static inline bool cpu_handle_halt(CPUState *cpu)
>> if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
>> && replay_interrupt()) {
>> X86CPU *x86_cpu = X86_CPU(cpu);
>> + qemu_mutex_lock_iothread();
>> apic_poll_irq(x86_cpu->apic_state);
>> cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
>> + qemu_mutex_unlock_iothread();
>> }
>> #endif
>> if (!cpu_has_work(cpu)) {
>> @@ -443,7 +446,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
>> #else
>> if (replay_exception()) {
>> CPUClass *cc = CPU_GET_CLASS(cpu);
>> + qemu_mutex_lock_iothread();
>> cc->do_interrupt(cpu);
>> + qemu_mutex_unlock_iothread();
>> cpu->exception_index = -1;
>> } else if (!replay_has_interrupt()) {
>> /* give a chance to iothread in replay mode */
>> @@ -469,9 +474,11 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
>> TranslationBlock **last_tb)
>> {
>> CPUClass *cc = CPU_GET_CLASS(cpu);
>> - int interrupt_request = cpu->interrupt_request;
>>
>> - if (unlikely(interrupt_request)) {
>> + if (unlikely(atomic_read(&cpu->interrupt_request))) {
>> + int interrupt_request;
>> + qemu_mutex_lock_iothread();
>> + interrupt_request = cpu->interrupt_request;
>> if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
>> /* Mask out external interrupts for this step. */
>> interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
>> @@ -526,7 +533,12 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
>> the program flow was changed */
>> *last_tb = NULL;
>> }
>> +
>> + /* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */
>> + qemu_mutex_unlock_iothread();
>> }
>> +
>> +
>> if (unlikely(atomic_read(&cpu->exit_request) || replay_has_interrupt())) {
>> atomic_set(&cpu->exit_request, 0);
>> cpu->exception_index = EXCP_INTERRUPT;
>> @@ -656,8 +668,12 @@ int cpu_exec(CPUState *cpu)
>> g_assert(cpu == current_cpu);
>> g_assert(cc == CPU_GET_CLASS(cpu));
>> #endif /* buggy compiler */
>> +
>> cpu->can_do_io = 1;
>> tb_lock_reset();
>> + if (qemu_mutex_iothread_locked()) {
>> + qemu_mutex_unlock_iothread();
>> + }
>> }
>> } /* for(;;) */
>>
>> diff --git a/cpus.c b/cpus.c
>> index 6d64199831..c48bc8d5b3 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -1026,8 +1026,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)
>> #endif /* _WIN32 */
>>
>> static QemuMutex qemu_global_mutex;
>> -static QemuCond qemu_io_proceeded_cond;
>> -static unsigned iothread_requesting_mutex;
>>
>> static QemuThread io_thread;
>>
>> @@ -1041,7 +1039,6 @@ void qemu_init_cpu_loop(void)
>> qemu_init_sigbus();
>> qemu_cond_init(&qemu_cpu_cond);
>> qemu_cond_init(&qemu_pause_cond);
>> - qemu_cond_init(&qemu_io_proceeded_cond);
>> qemu_mutex_init(&qemu_global_mutex);
>>
>> qemu_thread_get_self(&io_thread);
>> @@ -1084,10 +1081,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
>>
>> start_tcg_kick_timer();
>>
>> - while (iothread_requesting_mutex) {
>> - qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);
>> - }
>> -
>> CPU_FOREACH(cpu) {
>> qemu_wait_io_event_common(cpu);
>> }
>> @@ -1248,9 +1241,11 @@ static int tcg_cpu_exec(CPUState *cpu)
>> cpu->icount_decr.u16.low = decr;
>> cpu->icount_extra = count;
>> }
>> + qemu_mutex_unlock_iothread();
>> cpu_exec_start(cpu);
>> ret = cpu_exec(cpu);
>> cpu_exec_end(cpu);
>> + qemu_mutex_lock_iothread();
>> #ifdef CONFIG_PROFILER
>> tcg_time += profile_getclock() - ti;
>> #endif
>> @@ -1478,27 +1473,14 @@ bool qemu_mutex_iothread_locked(void)
>>
>> void qemu_mutex_lock_iothread(void)
>> {
>> - atomic_inc(&iothread_requesting_mutex);
>> - /* In the simple case there is no need to bump the VCPU thread out of
>> - * TCG code execution.
>> - */
>> - if (!tcg_enabled() || qemu_in_vcpu_thread() ||
>> - !first_cpu || !first_cpu->created) {
>> - qemu_mutex_lock(&qemu_global_mutex);
>> - atomic_dec(&iothread_requesting_mutex);
>> - } else {
>> - if (qemu_mutex_trylock(&qemu_global_mutex)) {
>> - qemu_cpu_kick_rr_cpu();
>> - qemu_mutex_lock(&qemu_global_mutex);
>> - }
>> - atomic_dec(&iothread_requesting_mutex);
>> - qemu_cond_broadcast(&qemu_io_proceeded_cond);
>> - }
>> + g_assert(!qemu_mutex_iothread_locked());
>
> How about using tcg_debug_assert here...
>
>> + qemu_mutex_lock(&qemu_global_mutex);
>> iothread_locked = true;
>> }
>>
>> void qemu_mutex_unlock_iothread(void)
>> {
>> + g_assert(qemu_mutex_iothread_locked());
>
> ... and here?
The iothread mutex is widely used in QEMU for bits other than TCG. In
fact I think it was introduced when KVM support was merged. As such I
don't want to remove these asserts from KVM mode.
It does mean this patch is slowly accreteing fixes for the BQL being
taken up by system emulation code though.
>
>> iothread_locked = false;
>> qemu_mutex_unlock(&qemu_global_mutex);
>> }
>> diff --git a/cputlb.c b/cputlb.c
>> index 6c39927455..1cc9d9da51 100644
>> --- a/cputlb.c
>> +++ b/cputlb.c
>> @@ -18,6 +18,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/main-loop.h"
>> #include "cpu.h"
>> #include "exec/exec-all.h"
>> #include "exec/memory.h"
>> @@ -495,6 +496,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
>> hwaddr physaddr = iotlbentry->addr;
>> MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
>> uint64_t val;
>> + bool locked = false;
>>
>> physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
>> cpu->mem_io_pc = retaddr;
>> @@ -503,7 +505,16 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
>> }
>>
>> cpu->mem_io_vaddr = addr;
>> +
>> + if (mr->global_locking) {
>> + qemu_mutex_lock_iothread();
>> + locked = true;
>> + }
>> memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs);
>> + if (locked) {
>> + qemu_mutex_unlock_iothread();
>> + }
>> +
>> return val;
>> }
>>
>> @@ -514,15 +525,23 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
>> CPUState *cpu = ENV_GET_CPU(env);
>> hwaddr physaddr = iotlbentry->addr;
>> MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
>> + bool locked = false;
>>
>> physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
>> if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {
>> cpu_io_recompile(cpu, retaddr);
>> }
>> -
>
> I am not really opposed but, I like having this line.. not sure why
> you are removing it? :)
Over zealous whitespace trimming, I can put it back ;-)
>
>> cpu->mem_io_vaddr = addr;
>> cpu->mem_io_pc = retaddr;
>> +
>> + if (mr->global_locking) {
>> + qemu_mutex_lock_iothread();
>> + locked = true;
>> + }
>> memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);
>> + if (locked) {
>> + qemu_mutex_unlock_iothread();
>> + }
>> }
>>
>> /* Return true if ADDR is present in the victim tlb, and has been copied
>> diff --git a/exec.c b/exec.c
>> index f2bed92b64..87cf0db91e 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -2133,9 +2133,9 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
>> }
>> cpu->watchpoint_hit = wp;
>>
>> - /* The tb_lock will be reset when cpu_loop_exit or
>> - * cpu_loop_exit_noexc longjmp back into the cpu_exec
>> - * main loop.
>> + /* Both tb_lock and iothread_mutex will be reset when
>> + * cpu_loop_exit or cpu_loop_exit_noexc longjmp
>> + * back into the cpu_exec main loop.
>> */
>> tb_lock();
>> tb_check_watchpoint(cpu);
>> @@ -2370,8 +2370,14 @@ static void io_mem_init(void)
>> memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);
>> memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
>> NULL, UINT64_MAX);
>> +
>> + /* io_mem_notdirty calls tb_invalidate_phys_page_fast,
>> + * which can be called without the iothread mutex.
>> + */
>> memory_region_init_io(&io_mem_notdirty, NULL, ¬dirty_mem_ops, NULL,
>> NULL, UINT64_MAX);
>> + memory_region_clear_global_locking(&io_mem_notdirty);
>> +
>> memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,
>> NULL, UINT64_MAX);
>> }
>> diff --git a/hw/core/irq.c b/hw/core/irq.c
>> index 49ff2e64fe..b98d1d69f5 100644
>> --- a/hw/core/irq.c
>> +++ b/hw/core/irq.c
>> @@ -22,6 +22,7 @@
>> * THE SOFTWARE.
>> */
>> #include "qemu/osdep.h"
>> +#include "qemu/main-loop.h"
>> #include "qemu-common.h"
>> #include "hw/irq.h"
>> #include "qom/object.h"
>> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
>> index 702e281dc8..b3c49b2c61 100644
>> --- a/hw/i386/kvmvapic.c
>> +++ b/hw/i386/kvmvapic.c
>> @@ -451,8 +451,8 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
>> resume_all_vcpus();
>>
>> if (!kvm_enabled()) {
>> - /* tb_lock will be reset when cpu_loop_exit_noexc longjmps
>> - * back into the cpu_exec loop. */
>> + /* Both tb_lock and iothread_mutex will be reset when
>> + * longjmps back into the cpu_exec loop. */
>
> missing cpu_loop_exit_noexc?
Well cpu_exec is the main loop, even if cpu_loop_exit_noexc is what
takes you there. I'll see if I can make it clearer:
/* Both tb_lock and iothread_mutex will be reset when
cpu_loop_exit_noexc longjmps back into the cpu_exec loop */
>
>> tb_lock();
>> tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
>> cpu_loop_exit_noexc(cs);
>> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
>> index a9ee7fddf9..2624d8d909 100644
>> --- a/hw/intc/arm_gicv3_cpuif.c
>> +++ b/hw/intc/arm_gicv3_cpuif.c
>> @@ -14,6 +14,7 @@
>>
>> #include "qemu/osdep.h"
>> #include "qemu/bitops.h"
>> +#include "qemu/main-loop.h"
>> #include "trace.h"
>> #include "gicv3_internal.h"
>> #include "cpu.h"
>> @@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
>> ARMCPU *cpu = ARM_CPU(cs->cpu);
>> CPUARMState *env = &cpu->env;
>>
>> + g_assert(qemu_mutex_iothread_locked());
>> +
>
> tcg_debug_assert()?
Depends if KVM can use this for emulation as well (which I think it can).
>
>> trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,
>> cs->hppi.grp, cs->hppi.prio);
>>
>> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
>> index 8945869009..59c3faa6c8 100644
>> --- a/hw/ppc/ppc.c
>> +++ b/hw/ppc/ppc.c
>> @@ -62,7 +62,16 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
>> {
>> CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>> - unsigned int old_pending = env->pending_interrupts;
>> + unsigned int old_pending;
>> + bool locked = false;
>> +
>> + /* We may already have the BQL if coming from the reset path */
>> + if (!qemu_mutex_iothread_locked()) {
>> + locked = true;
>> + qemu_mutex_lock_iothread();
>> + }
>> +
>> + old_pending = env->pending_interrupts;
>>
>> if (level) {
>> env->pending_interrupts |= 1 << n_IRQ;
>> @@ -80,9 +89,14 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
>> #endif
>> }
>>
>> +
>> LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
>> "req %08x\n", __func__, env, n_IRQ, level,
>> env->pending_interrupts, CPU(cpu)->interrupt_request);
>> +
>> + if (locked) {
>> + qemu_mutex_unlock_iothread();
>> + }
>> }
>>
>> /* PowerPC 6xx / 7xx internal IRQ controller */
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index a642e663d4..745743d64b 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1012,6 +1012,9 @@ static void emulate_spapr_hypercall(PowerPCCPU *cpu)
>> {
>> CPUPPCState *env = &cpu->env;
>>
>> + /* The TCG path should also be holding the BQL at this point */
>> + g_assert(qemu_mutex_iothread_locked());
>> +
>
> tcg_debug_assert()?
>
>> if (msr_pr) {
>> hcall_dprintf("Hypercall made with MSR[PR]=1\n");
>> env->gpr[3] = H_PRIVILEGE;
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 11db2015a4..1a06ae5938 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -325,6 +325,7 @@ struct CPUState {
>> bool unplug;
>> bool crash_occurred;
>> bool exit_request;
>> + /* updates protected by BQL */
>> uint32_t interrupt_request;
>> int singlestep_enabled;
>> int64_t icount_extra;
>> diff --git a/memory.c b/memory.c
>> index 2bfc37f65c..7d7b285e41 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -917,6 +917,8 @@ void memory_region_transaction_commit(void)
>> AddressSpace *as;
>>
>> assert(memory_region_transaction_depth);
>> + assert(qemu_mutex_iothread_locked());
>> +
>> --memory_region_transaction_depth;
>> if (!memory_region_transaction_depth) {
>> if (memory_region_update_pending) {
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 7f575879f6..bd77c05cd0 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -113,9 +113,19 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
>> error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
>> }
>>
>> +/* Resetting the IRQ comes from across the code base so we take the
>> + * BQL here if we need to. cpu_interrupt assumes it is held.*/
>> void cpu_reset_interrupt(CPUState *cpu, int mask)
>> {
>> + bool need_lock = !qemu_mutex_iothread_locked();
>> +
>> + if (need_lock) {
>> + qemu_mutex_lock_iothread();
>> + }
>> cpu->interrupt_request &= ~mask;
>> + if (need_lock) {
>> + qemu_mutex_unlock_iothread();
>> + }
>> }
>>
>> void cpu_exit(CPUState *cpu)
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 7111c8cf18..84d789be93 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -6693,6 +6693,12 @@ void arm_cpu_do_interrupt(CPUState *cs)
>> arm_cpu_do_interrupt_aarch32(cs);
>> }
>>
>> + /* Hooks may change global state so BQL should be held, also the
>> + * BQL needs to be held for any modification of
>> + * cs->interrupt_request.
>> + */
>> + g_assert(qemu_mutex_iothread_locked());
>> +
>> arm_call_el_change_hook(cpu);
>>
>> if (!kvm_enabled()) {
>> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
>> index ba796d898e..e1a883c595 100644
>> --- a/target/arm/op_helper.c
>> +++ b/target/arm/op_helper.c
>> @@ -18,6 +18,7 @@
>> */
>> #include "qemu/osdep.h"
>> #include "qemu/log.h"
>> +#include "qemu/main-loop.h"
>> #include "cpu.h"
>> #include "exec/helper-proto.h"
>> #include "internals.h"
>> @@ -487,7 +488,9 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)
>> */
>> env->regs[15] &= (env->thumb ? ~1 : ~3);
>>
>> + qemu_mutex_lock_iothread();
>> arm_call_el_change_hook(arm_env_get_cpu(env));
>> + qemu_mutex_unlock_iothread();
>> }
>>
>> /* Access to user mode registers from privileged modes. */
>> @@ -735,28 +738,58 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
>> {
>> const ARMCPRegInfo *ri = rip;
>>
>> - ri->writefn(env, ri, value);
>> + if (ri->type & ARM_CP_IO) {
>> + qemu_mutex_lock_iothread();
>> + ri->writefn(env, ri, value);
>> + qemu_mutex_unlock_iothread();
>> + } else {
>> + ri->writefn(env, ri, value);
>> + }
>> }
>>
>> uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)
>> {
>> const ARMCPRegInfo *ri = rip;
>> + uint32_t res;
>>
>> - return ri->readfn(env, ri);
>> + if (ri->type & ARM_CP_IO) {
>> + qemu_mutex_lock_iothread();
>> + res = ri->readfn(env, ri);
>> + qemu_mutex_unlock_iothread();
>> + } else {
>> + res = ri->readfn(env, ri);
>> + }
>> +
>> + return res;
>> }
>>
>> void HELPER(set_cp_reg64)(CPUARMState *env, void *rip, uint64_t value)
>> {
>> const ARMCPRegInfo *ri = rip;
>>
>> - ri->writefn(env, ri, value);
>> + if (ri->type & ARM_CP_IO) {
>> + qemu_mutex_lock_iothread();
>> + ri->writefn(env, ri, value);
>> + qemu_mutex_unlock_iothread();
>> + } else {
>> + ri->writefn(env, ri, value);
>> + }
>> }
>>
>> uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip)
>> {
>> const ARMCPRegInfo *ri = rip;
>> + uint64_t res;
>> +
>> + if (ri->type & ARM_CP_IO) {
>> + qemu_mutex_lock_iothread();
>> + res = ri->readfn(env, ri);
>> + qemu_mutex_unlock_iothread();
>> + } else {
>> + res = ri->readfn(env, ri);
>> + }
>>
>> - return ri->readfn(env, ri);
>> + return res;
>> }
>>
>> void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>> @@ -989,7 +1022,9 @@ void HELPER(exception_return)(CPUARMState *env)
>> cur_el, new_el, env->pc);
>> }
>>
>> + qemu_mutex_lock_iothread();
>> arm_call_el_change_hook(arm_env_get_cpu(env));
>> + qemu_mutex_unlock_iothread();
>>
>> return;
>>
>> diff --git a/target/i386/smm_helper.c b/target/i386/smm_helper.c
>> index 4dd6a2c544..f051a77c4a 100644
>> --- a/target/i386/smm_helper.c
>> +++ b/target/i386/smm_helper.c
>> @@ -18,6 +18,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/main-loop.h"
>> #include "cpu.h"
>> #include "exec/helper-proto.h"
>> #include "exec/log.h"
>> @@ -42,11 +43,14 @@ void helper_rsm(CPUX86State *env)
>> #define SMM_REVISION_ID 0x00020000
>> #endif
>>
>> +/* Called with iothread lock taken */
>> void cpu_smm_update(X86CPU *cpu)
>> {
>> CPUX86State *env = &cpu->env;
>> bool smm_enabled = (env->hflags & HF_SMM_MASK);
>>
>> + g_assert(qemu_mutex_iothread_locked());
>> +
>> if (cpu->smram) {
>> memory_region_set_enabled(cpu->smram, smm_enabled);
>> }
>> @@ -333,7 +337,10 @@ void helper_rsm(CPUX86State *env)
>> }
>> env->hflags2 &= ~HF2_SMM_INSIDE_NMI_MASK;
>> env->hflags &= ~HF_SMM_MASK;
>> +
>> + qemu_mutex_lock_iothread();
>> cpu_smm_update(cpu);
>> + qemu_mutex_unlock_iothread();
>>
>> qemu_log_mask(CPU_LOG_INT, "SMM: after RSM\n");
>> log_cpu_state_mask(CPU_LOG_INT, CPU(cpu), CPU_DUMP_CCOP);
>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
>> index c9604ea9c7..3cb942e8bb 100644
>> --- a/target/s390x/misc_helper.c
>> +++ b/target/s390x/misc_helper.c
>> @@ -25,6 +25,7 @@
>> #include "exec/helper-proto.h"
>> #include "sysemu/kvm.h"
>> #include "qemu/timer.h"
>> +#include "qemu/main-loop.h"
>> #include "exec/address-spaces.h"
>> #ifdef CONFIG_KVM
>> #include <linux/kvm.h>
>> @@ -109,11 +110,13 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
>> /* SCLP service call */
>> uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
>> {
>> + qemu_mutex_lock_iothread();
>> int r = sclp_service_call(env, r1, r2);
>> if (r < 0) {
>> program_interrupt(env, -r, 4);
>> - return 0;
>> + r = 0;
>> }
>> + qemu_mutex_unlock_iothread();
>> return r;
>> }
>>
>> diff --git a/translate-all.c b/translate-all.c
>> index 055436a676..41b36f04c6 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -55,6 +55,7 @@
>> #include "translate-all.h"
>> #include "qemu/bitmap.h"
>> #include "qemu/timer.h"
>> +#include "qemu/main-loop.h"
>> #include "exec/log.h"
>>
>> /* #define DEBUG_TB_INVALIDATE */
>> @@ -1521,7 +1522,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
>> #ifdef CONFIG_SOFTMMU
>> /* len must be <= 8 and start must be a multiple of len.
>> * Called via softmmu_template.h when code areas are written to with
>> - * tb_lock held.
>> + * iothread mutex not held.
>> */
>> void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
>> {
>
> Shouldn't this be both tb_lock and iothread mutex?
>
>> @@ -1723,7 +1724,10 @@ void tb_check_watchpoint(CPUState *cpu)
>>
>> #ifndef CONFIG_USER_ONLY
>> /* in deterministic execution mode, instructions doing device I/Os
>> - must be at the end of the TB */
>> + * must be at the end of the TB.
>> + *
>> + * Called by softmmu_template.h, with iothread mutex not held.
>> + */
>> void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
>> {
>> #if defined(TARGET_MIPS) || defined(TARGET_SH4)
>> @@ -1935,6 +1939,7 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
>>
>> void cpu_interrupt(CPUState *cpu, int mask)
>> {
>> + g_assert(qemu_mutex_iothread_locked());
>> cpu->interrupt_request |= mask;
>> cpu->tcg_exit_req = 1;
>> }
>> diff --git a/translate-common.c b/translate-common.c
>> index 5e989cdf70..d504dd0d33 100644
>> --- a/translate-common.c
>> +++ b/translate-common.c
>> @@ -21,6 +21,7 @@
>> #include "qemu-common.h"
>> #include "qom/cpu.h"
>> #include "sysemu/cpus.h"
>> +#include "qemu/main-loop.h"
>>
>> uintptr_t qemu_real_host_page_size;
>> intptr_t qemu_real_host_page_mask;
>> @@ -30,6 +31,7 @@ intptr_t qemu_real_host_page_mask;
>> static void tcg_handle_interrupt(CPUState *cpu, int mask)
>> {
>> int old_mask;
>> + g_assert(qemu_mutex_iothread_locked());
>>
>> old_mask = cpu->interrupt_request;
>> cpu->interrupt_request |= mask;
>> @@ -40,17 +42,16 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)
>> */
>> if (!qemu_cpu_is_self(cpu)) {
>> qemu_cpu_kick(cpu);
>> - return;
>> - }
>> -
>> - if (use_icount) {
>> - cpu->icount_decr.u16.high = 0xffff;
>> - if (!cpu->can_do_io
>> - && (mask & ~old_mask) != 0) {
>> - cpu_abort(cpu, "Raised interrupt while not in I/O function");
>> - }
>> } else {
>> - cpu->tcg_exit_req = 1;
>> + if (use_icount) {
>> + cpu->icount_decr.u16.high = 0xffff;
>> + if (!cpu->can_do_io
>> + && (mask & ~old_mask) != 0) {
>> + cpu_abort(cpu, "Raised interrupt while not in I/O function");
>> + }
>> + } else {
>> + cpu->tcg_exit_req = 1;
>> + }
>> }
>> }
>
> Reviewed-by: Pranith Kumar <bobby.prani@gmail.com>
Thanks.
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 08/25] tcg: drop global lock during TCG code execution
2017-01-30 9:57 ` Alex Bennée
@ 2017-01-30 16:52 ` Richard Henderson
0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2017-01-30 16:52 UTC (permalink / raw)
To: Alex Bennée, Pranith Kumar
Cc: mttcg, qemu-devel, fred.konrad, a.rigo, cota, nikunj, mark.burton,
pbonzini, jan.kiszka, serge.fdrv, peter.maydell, claudio.fontana,
bamvor.zhangjian, Peter Crosthwaite, Michael S. Tsirkin,
Eduardo Habkost, David Gibson, Alexander Graf,
open list:ARM cores, open list:PowerPC
On 01/30/2017 01:57 AM, Alex Bennée wrote:
>>> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
>>> index a9ee7fddf9..2624d8d909 100644
>>> --- a/hw/intc/arm_gicv3_cpuif.c
>>> +++ b/hw/intc/arm_gicv3_cpuif.c
>>> @@ -14,6 +14,7 @@
>>>
>>> #include "qemu/osdep.h"
>>> #include "qemu/bitops.h"
>>> +#include "qemu/main-loop.h"
>>> #include "trace.h"
>>> #include "gicv3_internal.h"
>>> #include "cpu.h"
>>> @@ -733,6 +734,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
>>> ARMCPU *cpu = ARM_CPU(cs->cpu);
>>> CPUARMState *env = &cpu->env;
>>>
>>> + g_assert(qemu_mutex_iothread_locked());
>>> +
>> tcg_debug_assert()?
> Depends if KVM can use this for emulation as well (which I think it can).
>
It probably could, but it shouldn't. Let's not leak tcg_* into hw/.
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap
2017-01-27 10:39 ` [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap Alex Bennée
@ 2017-01-31 23:09 ` Richard Henderson
2017-02-01 7:42 ` Alex Bennée
2017-02-01 11:03 ` Alex Bennée
0 siblings, 2 replies; 19+ messages in thread
From: Richard Henderson @ 2017-01-31 23:09 UTC (permalink / raw)
To: Alex Bennée, mttcg, qemu-devel, fred.konrad, a.rigo, cota,
bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, peter.maydell,
claudio.fontana, bamvor.zhangjian, Peter Crosthwaite,
Mark Cave-Ayland, Artyom Tarasenko, open list:ARM
On 01/27/2017 02:39 AM, Alex Bennée wrote:
> + for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>
> - tlb_debug("%d\n", mmu_idx);
> + if (test_bit(mmu_idx, &mmu_idx_bitmask)) {
> + tlb_debug("%d\n", mmu_idx);
>
> - memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
> - memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
> + memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
> + memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
> + }
Perhaps it doesn't matter since NB_MMU_MODES is so small but
for (; idxmap != 0; idxmap &= idxmap - 1) {
int mmu_idx = ctz32(idxmap);
...
}
iterates only for the bits that are set.
> -typedef enum ARMMMUIdx {
> - ARMMMUIdx_S12NSE0 = 0,
> - ARMMMUIdx_S12NSE1 = 1,
> - ARMMMUIdx_S1E2 = 2,
> - ARMMMUIdx_S1E3 = 3,
> - ARMMMUIdx_S1SE0 = 4,
> - ARMMMUIdx_S1SE1 = 5,
> - ARMMMUIdx_S2NS = 6,
> +typedef enum ARMMMUBitMap {
> + ARMMMUBit_S12NSE0 = 1 << 0,
> + ARMMMUBit_S12NSE1 = 1 << 1,
> + ARMMMUBit_S1E2 = 1 << 2,
> + ARMMMUBit_S1E3 = 1 << 3,
> + ARMMMUBit_S1SE0 = 1 << 4,
> + ARMMMUBit_S1SE1 = 1 << 5,
> + ARMMMUBit_S2NS = 1 << 6,
> /* Indexes below here don't have TLBs and are used only for AT system
> * instructions or for the first stage of an S12 page table walk.
> */
> - ARMMMUIdx_S1NSE0 = 7,
> - ARMMMUIdx_S1NSE1 = 8,
> -} ARMMMUIdx;
> + ARMMMUBit_S1NSE0 = 1 << 7,
> + ARMMMUBit_S1NSE1 = 1 << 8,
> +} ARMMMUBitMap;
>
> -#define MMU_USER_IDX 0
> +typedef int ARMMMUIdx;
> +
> +static inline ARMMMUIdx arm_mmu_bit_to_idx(ARMMMUBitMap bit)
> +{
> + g_assert(ctpop16(bit) == 1);
> + return ctz32(bit);
> +}
> +
> +static inline ARMMMUBitMap arm_mmu_idx_to_bit(ARMMMUIdx idx)
> +{
> + return 1 << idx;
> +}
I don't understand this redefinition though, causing...
> @@ -2109,13 +2109,13 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> /* stage 1 current state PL1: ATS1CPR, ATS1CPW */
> switch (el) {
> case 3:
> - mmu_idx = ARMMMUIdx_S1E3;
> + mmu_bit = ARMMMUBit_S1E3;
> break;
> case 2:
> - mmu_idx = ARMMMUIdx_S1NSE1;
> + mmu_bit = ARMMMUBit_S1NSE1;
> break;
> case 1:
> - mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
> + mmu_bit = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S1NSE1;
> break;
> default:
> g_assert_not_reached();
> @@ -2125,13 +2125,13 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> /* stage 1 current state PL0: ATS1CUR, ATS1CUW */
> switch (el) {
> case 3:
> - mmu_idx = ARMMMUIdx_S1SE0;
> + mmu_bit = ARMMMUBit_S1SE0;
> break;
> case 2:
> - mmu_idx = ARMMMUIdx_S1NSE0;
> + mmu_bit = ARMMMUBit_S1NSE0;
> break;
> case 1:
> - mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
> + mmu_bit = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S1NSE0;
> break;
> default:
> g_assert_not_reached();
> @@ -2139,17 +2139,17 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> break;
> case 4:
> /* stage 1+2 NonSecure PL1: ATS12NSOPR, ATS12NSOPW */
> - mmu_idx = ARMMMUIdx_S12NSE1;
> + mmu_bit = ARMMMUBit_S12NSE1;
> break;
> case 6:
> /* stage 1+2 NonSecure PL0: ATS12NSOUR, ATS12NSOUW */
> - mmu_idx = ARMMMUIdx_S12NSE0;
> + mmu_bit = ARMMMUBit_S12NSE0;
> break;
> default:
> g_assert_not_reached();
> }
>
> - par64 = do_ats_write(env, value, access_type, mmu_idx);
> + par64 = do_ats_write(env, value, access_type, arm_mmu_bit_to_idx(mmu_bit));
... this sort of churn, only to convert *back* to an index.
> @@ -2185,26 +2186,26 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
> case 0:
> switch (ri->opc1) {
> case 0: /* AT S1E1R, AT S1E1W */
> - mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
> + mmu_idx = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S1NSE1;
> break;
> case 4: /* AT S1E2R, AT S1E2W */
> - mmu_idx = ARMMMUIdx_S1E2;
> + mmu_idx = ARMMMUBit_S1E2;
> break;
> case 6: /* AT S1E3R, AT S1E3W */
> - mmu_idx = ARMMMUIdx_S1E3;
> + mmu_idx = ARMMMUBit_S1E3;
> break;
> default:
> g_assert_not_reached();
> }
> break;
> case 2: /* AT S1E0R, AT S1E0W */
> - mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
> + mmu_idx = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S1NSE0;
> break;
> case 4: /* AT S12E1R, AT S12E1W */
> - mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S12NSE1;
> + mmu_idx = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S12NSE1;
> break;
> case 6: /* AT S12E0R, AT S12E0W */
> - mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S12NSE0;
> + mmu_idx = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S12NSE0;
> break;
> default:
> g_assert_not_reached();
> @@ -2499,8 +2500,8 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
... and then there's this one where you don't rename the variable, and as far
as I can tell don't adjust the argument for do_ats_write.
Which is probably a mistake?
Just define both Idx and Bit symbols and be done with it.
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 24/25] target-arm: ensure all cross vCPUs TLB flushes complete
2017-01-27 10:39 ` [PATCH v8 24/25] target-arm: ensure all cross vCPUs TLB flushes complete Alex Bennée
@ 2017-01-31 23:37 ` Richard Henderson
0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2017-01-31 23:37 UTC (permalink / raw)
To: Alex Bennée, mttcg, qemu-devel, fred.konrad, a.rigo, cota,
bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, peter.maydell,
claudio.fontana, bamvor.zhangjian, open list:ARM
On 01/27/2017 02:39 AM, Alex Bennée wrote:
> Previously flushes on other vCPUs would only get serviced when they
> exited their TranslationBlocks. While this isn't overly problematic it
> violates the semantics of TLB flush from the point of view of source
> vCPU.
>
> To solve this we call the cputlb *_all_cpus() functions to do the
> flushes and ask it to ensure all flushes are completed before we start
> the next instruction. As this involves exiting the cpu_loop we need to
> ensure the PC is saved before the tlb helper functions are called.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 25/25] tcg: enable MTTCG by default for ARM on x86 hosts
2017-01-27 10:39 ` [PATCH v8 25/25] tcg: enable MTTCG by default for ARM on x86 hosts Alex Bennée
@ 2017-01-31 23:39 ` Richard Henderson
2017-02-01 7:44 ` Alex Bennée
0 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2017-01-31 23:39 UTC (permalink / raw)
To: Alex Bennée, mttcg, qemu-devel, fred.konrad, a.rigo, cota,
bobby.prani, nikunj
Cc: mark.burton, pbonzini, jan.kiszka, serge.fdrv, peter.maydell,
claudio.fontana, bamvor.zhangjian, open list:ARM
On 01/27/2017 02:39 AM, Alex Bennée wrote:
> +/* This defines the natural memory order supported by this
> + * architecture before guarantees made by various barrier
> + * instructions.
> + *
> + * The x86 has a pretty strong memory ordering which only really
> + * allows for some stores to be re-ordered after loads.
> + */
> +#include "tcg-mo.h"
> +
> +static inline int get_tcg_target_mo(void)
> +{
> + return TCG_MO_ALL & ~TCG_MO_LD_ST;
> +}
> +
> +#define TCG_TARGET_DEFAULT_MO get_tcg_target_mo()
> +
Why the inline function? The expression in the define would seem sufficient.
Otherwise,
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap
2017-01-31 23:09 ` Richard Henderson
@ 2017-02-01 7:42 ` Alex Bennée
2017-02-01 11:03 ` Alex Bennée
1 sibling, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2017-02-01 7:42 UTC (permalink / raw)
To: Richard Henderson
Cc: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj,
mark.burton, pbonzini, jan.kiszka, serge.fdrv, peter.maydell,
claudio.fontana, bamvor.zhangjian, Peter Crosthwaite,
Mark Cave-Ayland, Artyom Tarasenko, open list:ARM
Richard Henderson <rth@twiddle.net> writes:
> On 01/27/2017 02:39 AM, Alex Bennée wrote:
>> + for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>>
>> - tlb_debug("%d\n", mmu_idx);
>> + if (test_bit(mmu_idx, &mmu_idx_bitmask)) {
>> + tlb_debug("%d\n", mmu_idx);
>>
>> - memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
>> - memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
>> + memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
>> + memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
>> + }
>
> Perhaps it doesn't matter since NB_MMU_MODES is so small but
>
> for (; idxmap != 0; idxmap &= idxmap - 1) {
> int mmu_idx = ctz32(idxmap);
> ...
> }
>
> iterates only for the bits that are set.
>
>> -typedef enum ARMMMUIdx {
>> - ARMMMUIdx_S12NSE0 = 0,
>> - ARMMMUIdx_S12NSE1 = 1,
>> - ARMMMUIdx_S1E2 = 2,
>> - ARMMMUIdx_S1E3 = 3,
>> - ARMMMUIdx_S1SE0 = 4,
>> - ARMMMUIdx_S1SE1 = 5,
>> - ARMMMUIdx_S2NS = 6,
>> +typedef enum ARMMMUBitMap {
>> + ARMMMUBit_S12NSE0 = 1 << 0,
>> + ARMMMUBit_S12NSE1 = 1 << 1,
>> + ARMMMUBit_S1E2 = 1 << 2,
>> + ARMMMUBit_S1E3 = 1 << 3,
>> + ARMMMUBit_S1SE0 = 1 << 4,
>> + ARMMMUBit_S1SE1 = 1 << 5,
>> + ARMMMUBit_S2NS = 1 << 6,
>> /* Indexes below here don't have TLBs and are used only for AT system
>> * instructions or for the first stage of an S12 page table walk.
>> */
>> - ARMMMUIdx_S1NSE0 = 7,
>> - ARMMMUIdx_S1NSE1 = 8,
>> -} ARMMMUIdx;
>> + ARMMMUBit_S1NSE0 = 1 << 7,
>> + ARMMMUBit_S1NSE1 = 1 << 8,
>> +} ARMMMUBitMap;
>>
>> -#define MMU_USER_IDX 0
>> +typedef int ARMMMUIdx;
>> +
>> +static inline ARMMMUIdx arm_mmu_bit_to_idx(ARMMMUBitMap bit)
>> +{
>> + g_assert(ctpop16(bit) == 1);
>> + return ctz32(bit);
>> +}
>> +
>> +static inline ARMMMUBitMap arm_mmu_idx_to_bit(ARMMMUIdx idx)
>> +{
>> + return 1 << idx;
>> +}
>
> I don't understand this redefinition though, causing...
>
>> @@ -2109,13 +2109,13 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>> /* stage 1 current state PL1: ATS1CPR, ATS1CPW */
>> switch (el) {
>> case 3:
>> - mmu_idx = ARMMMUIdx_S1E3;
>> + mmu_bit = ARMMMUBit_S1E3;
>> break;
>> case 2:
>> - mmu_idx = ARMMMUIdx_S1NSE1;
>> + mmu_bit = ARMMMUBit_S1NSE1;
>> break;
>> case 1:
>> - mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
>> + mmu_bit = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S1NSE1;
>> break;
>> default:
>> g_assert_not_reached();
>> @@ -2125,13 +2125,13 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>> /* stage 1 current state PL0: ATS1CUR, ATS1CUW */
>> switch (el) {
>> case 3:
>> - mmu_idx = ARMMMUIdx_S1SE0;
>> + mmu_bit = ARMMMUBit_S1SE0;
>> break;
>> case 2:
>> - mmu_idx = ARMMMUIdx_S1NSE0;
>> + mmu_bit = ARMMMUBit_S1NSE0;
>> break;
>> case 1:
>> - mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
>> + mmu_bit = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S1NSE0;
>> break;
>> default:
>> g_assert_not_reached();
>> @@ -2139,17 +2139,17 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>> break;
>> case 4:
>> /* stage 1+2 NonSecure PL1: ATS12NSOPR, ATS12NSOPW */
>> - mmu_idx = ARMMMUIdx_S12NSE1;
>> + mmu_bit = ARMMMUBit_S12NSE1;
>> break;
>> case 6:
>> /* stage 1+2 NonSecure PL0: ATS12NSOUR, ATS12NSOUW */
>> - mmu_idx = ARMMMUIdx_S12NSE0;
>> + mmu_bit = ARMMMUBit_S12NSE0;
>> break;
>> default:
>> g_assert_not_reached();
>> }
>>
>> - par64 = do_ats_write(env, value, access_type, mmu_idx);
>> + par64 = do_ats_write(env, value, access_type, arm_mmu_bit_to_idx(mmu_bit));
>
> ... this sort of churn, only to convert *back* to an index.
Yeah I've dropped this is v9, ARM now justs does 1 << ARMMMUIdx_foo at
the tlb_flush call sites.
>
>> @@ -2185,26 +2186,26 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
>> case 0:
>> switch (ri->opc1) {
>> case 0: /* AT S1E1R, AT S1E1W */
>> - mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
>> + mmu_idx = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S1NSE1;
>> break;
>> case 4: /* AT S1E2R, AT S1E2W */
>> - mmu_idx = ARMMMUIdx_S1E2;
>> + mmu_idx = ARMMMUBit_S1E2;
>> break;
>> case 6: /* AT S1E3R, AT S1E3W */
>> - mmu_idx = ARMMMUIdx_S1E3;
>> + mmu_idx = ARMMMUBit_S1E3;
>> break;
>> default:
>> g_assert_not_reached();
>> }
>> break;
>> case 2: /* AT S1E0R, AT S1E0W */
>> - mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
>> + mmu_idx = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S1NSE0;
>> break;
>> case 4: /* AT S12E1R, AT S12E1W */
>> - mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S12NSE1;
>> + mmu_idx = secure ? ARMMMUBit_S1SE1 : ARMMMUBit_S12NSE1;
>> break;
>> case 6: /* AT S12E0R, AT S12E0W */
>> - mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S12NSE0;
>> + mmu_idx = secure ? ARMMMUBit_S1SE0 : ARMMMUBit_S12NSE0;
>> break;
>> default:
>> g_assert_not_reached();
>> @@ -2499,8 +2500,8 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>
> ... and then there's this one where you don't rename the variable, and as far
> as I can tell don't adjust the argument for do_ats_write.
>
> Which is probably a mistake?
>
> Just define both Idx and Bit symbols and be done with it.
>
>
> r~
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 25/25] tcg: enable MTTCG by default for ARM on x86 hosts
2017-01-31 23:39 ` Richard Henderson
@ 2017-02-01 7:44 ` Alex Bennée
0 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2017-02-01 7:44 UTC (permalink / raw)
To: Richard Henderson
Cc: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj,
mark.burton, pbonzini, jan.kiszka, serge.fdrv, peter.maydell,
claudio.fontana, bamvor.zhangjian, open list:ARM
Richard Henderson <rth@twiddle.net> writes:
> On 01/27/2017 02:39 AM, Alex Bennée wrote:
>> +/* This defines the natural memory order supported by this
>> + * architecture before guarantees made by various barrier
>> + * instructions.
>> + *
>> + * The x86 has a pretty strong memory ordering which only really
>> + * allows for some stores to be re-ordered after loads.
>> + */
>> +#include "tcg-mo.h"
>> +
>> +static inline int get_tcg_target_mo(void)
>> +{
>> + return TCG_MO_ALL & ~TCG_MO_LD_ST;
>> +}
>> +
>> +#define TCG_TARGET_DEFAULT_MO get_tcg_target_mo()
>> +
>
> Why the inline function? The expression in the define would seem sufficient.
> Otherwise,
Good point. It was just to get around the definition at pre-processor and
compile time.
>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
>
>
> r~
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap
2017-01-31 23:09 ` Richard Henderson
2017-02-01 7:42 ` Alex Bennée
@ 2017-02-01 11:03 ` Alex Bennée
2017-02-01 18:03 ` Richard Henderson
1 sibling, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2017-02-01 11:03 UTC (permalink / raw)
To: Richard Henderson
Cc: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj,
mark.burton, pbonzini, jan.kiszka, serge.fdrv, peter.maydell,
claudio.fontana, bamvor.zhangjian, Peter Crosthwaite,
Mark Cave-Ayland, Artyom Tarasenko, open list:ARM
Richard Henderson <rth@twiddle.net> writes:
> On 01/27/2017 02:39 AM, Alex Bennée wrote:
>> + for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>>
>> - tlb_debug("%d\n", mmu_idx);
>> + if (test_bit(mmu_idx, &mmu_idx_bitmask)) {
>> + tlb_debug("%d\n", mmu_idx);
>>
>> - memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
>> - memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
>> + memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
>> + memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
>> + }
>
> Perhaps it doesn't matter since NB_MMU_MODES is so small but
>
> for (; idxmap != 0; idxmap &= idxmap - 1) {
> int mmu_idx = ctz32(idxmap);
> ...
> }
Perhaps but if it is OK with you I'll skip this optimisation for now? We
are basically in the slow path by this point and for clarity I'd prefer
to keep it as is.
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap
2017-02-01 11:03 ` Alex Bennée
@ 2017-02-01 18:03 ` Richard Henderson
0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2017-02-01 18:03 UTC (permalink / raw)
To: Alex Bennée
Cc: mttcg, qemu-devel, fred.konrad, a.rigo, cota, bobby.prani, nikunj,
mark.burton, pbonzini, jan.kiszka, serge.fdrv, peter.maydell,
claudio.fontana, bamvor.zhangjian, Peter Crosthwaite,
Mark Cave-Ayland, Artyom Tarasenko, open list:ARM
On 02/01/2017 03:03 AM, Alex Bennée wrote:
>
> Richard Henderson <rth@twiddle.net> writes:
>
>> On 01/27/2017 02:39 AM, Alex Bennée wrote:
>>> + for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>>>
>>> - tlb_debug("%d\n", mmu_idx);
>>> + if (test_bit(mmu_idx, &mmu_idx_bitmask)) {
>>> + tlb_debug("%d\n", mmu_idx);
>>>
>>> - memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
>>> - memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
>>> + memset(env->tlb_table[mmu_idx], -1, sizeof(env->tlb_table[0]));
>>> + memset(env->tlb_v_table[mmu_idx], -1, sizeof(env->tlb_v_table[0]));
>>> + }
>>
>> Perhaps it doesn't matter since NB_MMU_MODES is so small but
>>
>> for (; idxmap != 0; idxmap &= idxmap - 1) {
>> int mmu_idx = ctz32(idxmap);
>> ...
>> }
>
> Perhaps but if it is OK with you I'll skip this optimisation for now? We
> are basically in the slow path by this point and for clarity I'd prefer
> to keep it as is.
That's fine.
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-02-01 18:03 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20170127103922.19658-1-alex.bennee@linaro.org>
2017-01-27 10:39 ` [PATCH v8 08/25] tcg: drop global lock during TCG code execution Alex Bennée
2017-01-29 21:33 ` Pranith Kumar
2017-01-30 9:57 ` Alex Bennée
2017-01-30 16:52 ` Richard Henderson
2017-01-27 10:39 ` [PATCH v8 16/25] cputlb and arm/sparc targets: convert mmuidx flushes from varg to bitmap Alex Bennée
2017-01-31 23:09 ` Richard Henderson
2017-02-01 7:42 ` Alex Bennée
2017-02-01 11:03 ` Alex Bennée
2017-02-01 18:03 ` Richard Henderson
2017-01-27 10:39 ` [PATCH v8 20/25] target-arm/powerctl: defer cpu reset work to CPU context Alex Bennée
2017-01-27 10:39 ` [PATCH v8 21/25] target-arm: don't generate WFE/YIELD calls for MTTCG Alex Bennée
2017-01-27 10:39 ` [PATCH v8 22/25] target-arm/cpu.h: make ARM_CP defined consistent Alex Bennée
2017-01-27 10:39 ` [PATCH v8 23/25] target-arm: introduce ARM_CP_EXIT_PC Alex Bennée
2017-01-27 10:39 ` [PATCH v8 24/25] target-arm: ensure all cross vCPUs TLB flushes complete Alex Bennée
2017-01-31 23:37 ` Richard Henderson
2017-01-27 10:39 ` [PATCH v8 25/25] tcg: enable MTTCG by default for ARM on x86 hosts Alex Bennée
2017-01-31 23:39 ` Richard Henderson
2017-02-01 7:44 ` Alex Bennée
[not found] <20170127103505.18606-1-alex.bennee@linaro.org>
2017-01-27 10:34 ` [PATCH v8 08/25] tcg: drop global lock during TCG code execution Alex Bennée
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox