* [PATCH 02/35] target/i386: use cpu_reset_interrupt
2018-09-17 16:30 [PATCH 00/35] exec: drop BQL from interrupt handling Emilio G. Cota
@ 2018-09-17 16:30 ` Emilio G. Cota
2018-09-18 20:47 ` Richard Henderson
2018-09-17 16:30 ` [PATCH 11/35] target/i386: access cpu->interrupt_request with atomics Emilio G. Cota
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Emilio G. Cota @ 2018-09-17 16:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Eduardo Habkost, kvm,
Richard Henderson
From: Paolo Bonzini <pbonzini@redhat.com>
It will be changed to an atomic operation soon.
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
target/i386/hax-all.c | 4 ++--
target/i386/hvf/x86hvf.c | 8 ++++----
target/i386/kvm.c | 14 +++++++-------
target/i386/seg_helper.c | 13 ++++++-------
target/i386/svm_helper.c | 2 +-
target/i386/whpx-all.c | 10 +++++-----
6 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index d2e512856b..ae8b678db0 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -433,7 +433,7 @@ static int hax_vcpu_interrupt(CPUArchState *env)
irq = cpu_get_pic_interrupt(env);
if (irq >= 0) {
hax_inject_interrupt(env, irq);
- cpu->interrupt_request &= ~CPU_INTERRUPT_HARD;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
}
}
@@ -483,7 +483,7 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
cpu->halted = 0;
if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
- cpu->interrupt_request &= ~CPU_INTERRUPT_POLL;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
apic_poll_irq(x86_cpu->apic_state);
}
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index 6c88939b96..3ac796b885 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -402,7 +402,7 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
if (cpu_state->interrupt_request & CPU_INTERRUPT_NMI) {
if (!(env->hflags2 & HF2_NMI_MASK) && !(info & VMCS_INTR_VALID)) {
- cpu_state->interrupt_request &= ~CPU_INTERRUPT_NMI;
+ cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_NMI);
info = VMCS_INTR_VALID | VMCS_INTR_T_NMI | NMI_VEC;
wvmcs(cpu_state->hvf_fd, VMCS_ENTRY_INTR_INFO, info);
} else {
@@ -414,7 +414,7 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
(cpu_state->interrupt_request & CPU_INTERRUPT_HARD) &&
(EFLAGS(env) & IF_MASK) && !(info & VMCS_INTR_VALID)) {
int line = cpu_get_pic_interrupt(&x86cpu->env);
- cpu_state->interrupt_request &= ~CPU_INTERRUPT_HARD;
+ cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_HARD);
if (line >= 0) {
wvmcs(cpu_state->hvf_fd, VMCS_ENTRY_INTR_INFO, line |
VMCS_INTR_VALID | VMCS_INTR_T_HWINTR);
@@ -440,7 +440,7 @@ int hvf_process_events(CPUState *cpu_state)
}
if (cpu_state->interrupt_request & CPU_INTERRUPT_POLL) {
- cpu_state->interrupt_request &= ~CPU_INTERRUPT_POLL;
+ cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_POLL);
apic_poll_irq(cpu->apic_state);
}
if (((cpu_state->interrupt_request & CPU_INTERRUPT_HARD) &&
@@ -453,7 +453,7 @@ int hvf_process_events(CPUState *cpu_state)
do_cpu_sipi(cpu);
}
if (cpu_state->interrupt_request & CPU_INTERRUPT_TPR) {
- cpu_state->interrupt_request &= ~CPU_INTERRUPT_TPR;
+ cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_TPR);
hvf_cpu_synchronize_state(cpu_state);
apic_handle_tpr_access_report(cpu->apic_state, env->eip,
env->tpr_access_type);
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 0b2a07d3a4..5dd66809b0 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2709,7 +2709,7 @@ static int kvm_put_vcpu_events(X86CPU *cpu, int level)
*/
events.smi.pending = cs->interrupt_request & CPU_INTERRUPT_SMI;
events.smi.latched_init = cs->interrupt_request & CPU_INTERRUPT_INIT;
- cs->interrupt_request &= ~(CPU_INTERRUPT_INIT | CPU_INTERRUPT_SMI);
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_INIT | CPU_INTERRUPT_SMI);
} else {
/* Keep these in cs->interrupt_request. */
events.smi.pending = 0;
@@ -3005,7 +3005,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
if (cpu->interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
if (cpu->interrupt_request & CPU_INTERRUPT_NMI) {
qemu_mutex_lock_iothread();
- cpu->interrupt_request &= ~CPU_INTERRUPT_NMI;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_NMI);
qemu_mutex_unlock_iothread();
DPRINTF("injected NMI\n");
ret = kvm_vcpu_ioctl(cpu, KVM_NMI);
@@ -3016,7 +3016,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
}
if (cpu->interrupt_request & CPU_INTERRUPT_SMI) {
qemu_mutex_lock_iothread();
- cpu->interrupt_request &= ~CPU_INTERRUPT_SMI;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_SMI);
qemu_mutex_unlock_iothread();
DPRINTF("injected SMI\n");
ret = kvm_vcpu_ioctl(cpu, KVM_SMI);
@@ -3052,7 +3052,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
(env->eflags & IF_MASK)) {
int irq;
- cpu->interrupt_request &= ~CPU_INTERRUPT_HARD;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
irq = cpu_get_pic_interrupt(env);
if (irq >= 0) {
struct kvm_interrupt intr;
@@ -3123,7 +3123,7 @@ int kvm_arch_process_async_events(CPUState *cs)
/* We must not raise CPU_INTERRUPT_MCE if it's not supported. */
assert(env->mcg_cap);
- cs->interrupt_request &= ~CPU_INTERRUPT_MCE;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_MCE);
kvm_cpu_synchronize_state(cs);
@@ -3153,7 +3153,7 @@ int kvm_arch_process_async_events(CPUState *cs)
}
if (cs->interrupt_request & CPU_INTERRUPT_POLL) {
- cs->interrupt_request &= ~CPU_INTERRUPT_POLL;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL);
apic_poll_irq(cpu->apic_state);
}
if (((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
@@ -3166,7 +3166,7 @@ int kvm_arch_process_async_events(CPUState *cs)
do_cpu_sipi(cpu);
}
if (cs->interrupt_request & CPU_INTERRUPT_TPR) {
- cs->interrupt_request &= ~CPU_INTERRUPT_TPR;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_TPR);
kvm_cpu_synchronize_state(cs);
apic_handle_tpr_access_report(cpu->apic_state, env->eip,
env->tpr_access_type);
diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
index d1cbc6ebf0..0dd85329db 100644
--- a/target/i386/seg_helper.c
+++ b/target/i386/seg_helper.c
@@ -1323,7 +1323,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
#if !defined(CONFIG_USER_ONLY)
if (interrupt_request & CPU_INTERRUPT_POLL) {
- cs->interrupt_request &= ~CPU_INTERRUPT_POLL;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL);
apic_poll_irq(cpu->apic_state);
/* Don't process multiple interrupt requests in a single call.
This is required to make icount-driven execution deterministic. */
@@ -1337,18 +1337,18 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
if ((interrupt_request & CPU_INTERRUPT_SMI) &&
!(env->hflags & HF_SMM_MASK)) {
cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0, 0);
- cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_SMI);
do_smm_enter(cpu);
ret = true;
} else if ((interrupt_request & CPU_INTERRUPT_NMI) &&
!(env->hflags2 & HF2_NMI_MASK)) {
cpu_svm_check_intercept_param(env, SVM_EXIT_NMI, 0, 0);
- cs->interrupt_request &= ~CPU_INTERRUPT_NMI;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_NMI);
env->hflags2 |= HF2_NMI_MASK;
do_interrupt_x86_hardirq(env, EXCP02_NMI, 1);
ret = true;
} else if (interrupt_request & CPU_INTERRUPT_MCE) {
- cs->interrupt_request &= ~CPU_INTERRUPT_MCE;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_MCE);
do_interrupt_x86_hardirq(env, EXCP12_MCHK, 0);
ret = true;
} else if ((interrupt_request & CPU_INTERRUPT_HARD) &&
@@ -1359,8 +1359,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
!(env->hflags & HF_INHIBIT_IRQ_MASK))))) {
int intno;
cpu_svm_check_intercept_param(env, SVM_EXIT_INTR, 0, 0);
- cs->interrupt_request &= ~(CPU_INTERRUPT_HARD |
- CPU_INTERRUPT_VIRQ);
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD | CPU_INTERRUPT_VIRQ);
intno = cpu_get_pic_interrupt(env);
qemu_log_mask(CPU_LOG_TB_IN_ASM,
"Servicing hardware INT=0x%02x\n", intno);
@@ -1380,7 +1379,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
qemu_log_mask(CPU_LOG_TB_IN_ASM,
"Servicing virtual hardware INT=0x%02x\n", intno);
do_interrupt_x86_hardirq(env, intno, 1);
- cs->interrupt_request &= ~CPU_INTERRUPT_VIRQ;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);
ret = true;
#endif
}
diff --git a/target/i386/svm_helper.c b/target/i386/svm_helper.c
index 342ece082f..c532639574 100644
--- a/target/i386/svm_helper.c
+++ b/target/i386/svm_helper.c
@@ -700,7 +700,7 @@ void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
env->hflags &= ~HF_SVMI_MASK;
env->intercept = 0;
env->intercept_exceptions = 0;
- cs->interrupt_request &= ~CPU_INTERRUPT_VIRQ;
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);
env->tsc_offset = 0;
env->gdt.base = x86_ldq_phys(cs, env->vm_hsave + offsetof(struct vmcb,
diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 57e53e1f1f..d9428dc987 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -728,14 +728,14 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
if (!vcpu->interruption_pending &&
cpu->interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
if (cpu->interrupt_request & CPU_INTERRUPT_NMI) {
- cpu->interrupt_request &= ~CPU_INTERRUPT_NMI;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_NMI);
vcpu->interruptable = false;
new_int.InterruptionType = WHvX64PendingNmi;
new_int.InterruptionPending = 1;
new_int.InterruptionVector = 2;
}
if (cpu->interrupt_request & CPU_INTERRUPT_SMI) {
- cpu->interrupt_request &= ~CPU_INTERRUPT_SMI;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_SMI);
}
}
@@ -758,7 +758,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
vcpu->interruptable && (env->eflags & IF_MASK)) {
assert(!new_int.InterruptionPending);
if (cpu->interrupt_request & CPU_INTERRUPT_HARD) {
- cpu->interrupt_request &= ~CPU_INTERRUPT_HARD;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
irq = cpu_get_pic_interrupt(env);
if (irq >= 0) {
new_int.InterruptionType = WHvX64PendingInterrupt;
@@ -850,7 +850,7 @@ static void whpx_vcpu_process_async_events(CPUState *cpu)
}
if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
- cpu->interrupt_request &= ~CPU_INTERRUPT_POLL;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
apic_poll_irq(x86_cpu->apic_state);
}
@@ -868,7 +868,7 @@ static void whpx_vcpu_process_async_events(CPUState *cpu)
}
if (cpu->interrupt_request & CPU_INTERRUPT_TPR) {
- cpu->interrupt_request &= ~CPU_INTERRUPT_TPR;
+ cpu_reset_interrupt(cpu, CPU_INTERRUPT_TPR);
if (!cpu->vcpu_dirty) {
whpx_get_registers(cpu);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 02/35] target/i386: use cpu_reset_interrupt
2018-09-17 16:30 ` [PATCH 02/35] target/i386: use cpu_reset_interrupt Emilio G. Cota
@ 2018-09-18 20:47 ` Richard Henderson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2018-09-18 20:47 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Eduardo Habkost, kvm
On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> It will be changed to an atomic operation soon.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
> target/i386/hax-all.c | 4 ++--
> target/i386/hvf/x86hvf.c | 8 ++++----
> target/i386/kvm.c | 14 +++++++-------
> target/i386/seg_helper.c | 13 ++++++-------
> target/i386/svm_helper.c | 2 +-
> target/i386/whpx-all.c | 10 +++++-----
> 6 files changed, 25 insertions(+), 26 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 11/35] target/i386: access cpu->interrupt_request with atomics
2018-09-17 16:30 [PATCH 00/35] exec: drop BQL from interrupt handling Emilio G. Cota
2018-09-17 16:30 ` [PATCH 02/35] target/i386: use cpu_reset_interrupt Emilio G. Cota
@ 2018-09-17 16:30 ` Emilio G. Cota
2018-09-18 21:04 ` Richard Henderson
2018-09-17 16:31 ` [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt Emilio G. Cota
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Emilio G. Cota @ 2018-09-17 16:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Eduardo Habkost, kvm,
Richard Henderson
From: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
target/i386/cpu.c | 7 ++++---
target/i386/helper.c | 4 ++--
target/i386/kvm.c | 44 +++++++++++++++++++++++-----------------
target/i386/svm_helper.c | 4 ++--
4 files changed, 33 insertions(+), 26 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f24295e6e4..f98e6e4318 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5433,15 +5433,16 @@ static bool x86_cpu_has_work(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = &cpu->env;
+ int interrupt_request = atomic_read(&cs->interrupt_request);
- return ((cs->interrupt_request & (CPU_INTERRUPT_HARD |
+ return ((interrupt_request & (CPU_INTERRUPT_HARD |
CPU_INTERRUPT_POLL)) &&
(env->eflags & IF_MASK)) ||
- (cs->interrupt_request & (CPU_INTERRUPT_NMI |
+ (interrupt_request & (CPU_INTERRUPT_NMI |
CPU_INTERRUPT_INIT |
CPU_INTERRUPT_SIPI |
CPU_INTERRUPT_MCE)) ||
- ((cs->interrupt_request & CPU_INTERRUPT_SMI) &&
+ ((interrupt_request & CPU_INTERRUPT_SMI) &&
!(env->hflags & HF_SMM_MASK));
}
diff --git a/target/i386/helper.c b/target/i386/helper.c
index e695f8ba7a..ee9f24d853 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -1035,12 +1035,12 @@ void do_cpu_init(X86CPU *cpu)
CPUState *cs = CPU(cpu);
CPUX86State *env = &cpu->env;
CPUX86State *save = g_new(CPUX86State, 1);
- int sipi = cs->interrupt_request & CPU_INTERRUPT_SIPI;
+ int sipi = atomic_read(&cs->interrupt_request) & CPU_INTERRUPT_SIPI;
*save = *env;
cpu_reset(cs);
- cs->interrupt_request = sipi;
+ atomic_mb_set(&cs->interrupt_request, sipi);
memcpy(&env->start_init_save, &save->start_init_save,
offsetof(CPUX86State, end_init_save) -
offsetof(CPUX86State, start_init_save));
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 5dd66809b0..e40c8d5461 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2707,8 +2707,10 @@ static int kvm_put_vcpu_events(X86CPU *cpu, int level)
/* As soon as these are moved to the kernel, remove them
* from cs->interrupt_request.
*/
- events.smi.pending = cs->interrupt_request & CPU_INTERRUPT_SMI;
- events.smi.latched_init = cs->interrupt_request & CPU_INTERRUPT_INIT;
+ uint32_t interrupt_request = atomic_read(&cs->interrupt_request);
+
+ events.smi.pending = interrupt_request & CPU_INTERRUPT_SMI;
+ events.smi.latched_init = interrupt_request & CPU_INTERRUPT_INIT;
cpu_reset_interrupt(cs, CPU_INTERRUPT_INIT | CPU_INTERRUPT_SMI);
} else {
/* Keep these in cs->interrupt_request. */
@@ -2999,11 +3001,12 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
{
X86CPU *x86_cpu = X86_CPU(cpu);
CPUX86State *env = &x86_cpu->env;
+ int interrupt_request = atomic_read(&cpu->interrupt_request);
int ret;
/* Inject NMI */
- if (cpu->interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
- if (cpu->interrupt_request & CPU_INTERRUPT_NMI) {
+ if (interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
+ if (interrupt_request & CPU_INTERRUPT_NMI) {
qemu_mutex_lock_iothread();
cpu_reset_interrupt(cpu, CPU_INTERRUPT_NMI);
qemu_mutex_unlock_iothread();
@@ -3014,7 +3017,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
strerror(-ret));
}
}
- if (cpu->interrupt_request & CPU_INTERRUPT_SMI) {
+ if (atomic_read(&cpu->interrupt_request) & CPU_INTERRUPT_SMI) {
qemu_mutex_lock_iothread();
cpu_reset_interrupt(cpu, CPU_INTERRUPT_SMI);
qemu_mutex_unlock_iothread();
@@ -3035,12 +3038,12 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
* or (for userspace APIC, but it is cheap to combine the checks here)
* pending TPR access reports.
*/
- if (cpu->interrupt_request & (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
- if ((cpu->interrupt_request & CPU_INTERRUPT_INIT) &&
+ if (interrupt_request & (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
+ if ((interrupt_request & CPU_INTERRUPT_INIT) &&
!(env->hflags & HF_SMM_MASK)) {
cpu->exit_request = 1;
}
- if (cpu->interrupt_request & CPU_INTERRUPT_TPR) {
+ if (interrupt_request & CPU_INTERRUPT_TPR) {
cpu->exit_request = 1;
}
}
@@ -3048,11 +3051,12 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
if (!kvm_pic_in_kernel()) {
/* Try to inject an interrupt if the guest can accept it */
if (run->ready_for_interrupt_injection &&
- (cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
+ (interrupt_request & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK)) {
int irq;
cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
+ interrupt_request &= ~CPU_INTERRUPT_HARD;
irq = cpu_get_pic_interrupt(env);
if (irq >= 0) {
struct kvm_interrupt intr;
@@ -3072,7 +3076,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
* interrupt, request an interrupt window exit. This will
* cause a return to userspace as soon as the guest is ready to
* receive interrupts. */
- if ((cpu->interrupt_request & CPU_INTERRUPT_HARD)) {
+ if (interrupt_request & CPU_INTERRUPT_HARD) {
run->request_interrupt_window = 1;
} else {
run->request_interrupt_window = 0;
@@ -3118,8 +3122,9 @@ int kvm_arch_process_async_events(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = &cpu->env;
+ int interrupt_request = atomic_read(&cs->interrupt_request);
- if (cs->interrupt_request & CPU_INTERRUPT_MCE) {
+ if (interrupt_request & CPU_INTERRUPT_MCE) {
/* We must not raise CPU_INTERRUPT_MCE if it's not supported. */
assert(env->mcg_cap);
@@ -3142,7 +3147,7 @@ int kvm_arch_process_async_events(CPUState *cs)
}
}
- if ((cs->interrupt_request & CPU_INTERRUPT_INIT) &&
+ if ((interrupt_request & CPU_INTERRUPT_INIT) &&
!(env->hflags & HF_SMM_MASK)) {
kvm_cpu_synchronize_state(cs);
do_cpu_init(cpu);
@@ -3152,20 +3157,20 @@ int kvm_arch_process_async_events(CPUState *cs)
return 0;
}
- if (cs->interrupt_request & CPU_INTERRUPT_POLL) {
+ if (interrupt_request & CPU_INTERRUPT_POLL) {
cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL);
apic_poll_irq(cpu->apic_state);
}
- if (((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+ if (((interrupt_request & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK)) ||
- (cs->interrupt_request & CPU_INTERRUPT_NMI)) {
+ (interrupt_request & CPU_INTERRUPT_NMI)) {
cs->halted = 0;
}
- if (cs->interrupt_request & CPU_INTERRUPT_SIPI) {
+ if (interrupt_request & CPU_INTERRUPT_SIPI) {
kvm_cpu_synchronize_state(cs);
do_cpu_sipi(cpu);
}
- if (cs->interrupt_request & CPU_INTERRUPT_TPR) {
+ if (interrupt_request & CPU_INTERRUPT_TPR) {
cpu_reset_interrupt(cs, CPU_INTERRUPT_TPR);
kvm_cpu_synchronize_state(cs);
apic_handle_tpr_access_report(cpu->apic_state, env->eip,
@@ -3179,10 +3184,11 @@ static int kvm_handle_halt(X86CPU *cpu)
{
CPUState *cs = CPU(cpu);
CPUX86State *env = &cpu->env;
+ int interrupt_request = atomic_read(&cs->interrupt_request);
- if (!((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+ if (!((interrupt_request & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK)) &&
- !(cs->interrupt_request & CPU_INTERRUPT_NMI)) {
+ !(interrupt_request & CPU_INTERRUPT_NMI)) {
cs->halted = 1;
return EXCP_HLT;
}
diff --git a/target/i386/svm_helper.c b/target/i386/svm_helper.c
index c532639574..e18e53c869 100644
--- a/target/i386/svm_helper.c
+++ b/target/i386/svm_helper.c
@@ -316,7 +316,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
if (int_ctl & V_IRQ_MASK) {
CPUState *cs = CPU(x86_env_get_cpu(env));
- cs->interrupt_request |= CPU_INTERRUPT_VIRQ;
+ atomic_or(&cs->interrupt_request, CPU_INTERRUPT_VIRQ);
}
/* maybe we need to inject an event */
@@ -674,7 +674,7 @@ void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
env->vm_vmcb + offsetof(struct vmcb, control.int_ctl));
int_ctl &= ~(V_TPR_MASK | V_IRQ_MASK);
int_ctl |= env->v_tpr & V_TPR_MASK;
- if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
+ if (atomic_read(&cs->interrupt_request) & CPU_INTERRUPT_VIRQ) {
int_ctl |= V_IRQ_MASK;
}
x86_stl_phys(cs,
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 11/35] target/i386: access cpu->interrupt_request with atomics
2018-09-17 16:30 ` [PATCH 11/35] target/i386: access cpu->interrupt_request with atomics Emilio G. Cota
@ 2018-09-18 21:04 ` Richard Henderson
2018-09-19 15:02 ` Emilio G. Cota
0 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2018-09-18 21:04 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Eduardo Habkost, kvm
On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> cpu_reset(cs);
> - cs->interrupt_request = sipi;
> + atomic_mb_set(&cs->interrupt_request, sipi);
> memcpy(&env->start_init_save, &save->start_init_save,
Why does this need a memory barrier?
Anyway, I think a bare mechanical conversion would be best
for the first patch and then extra barriers added separately
and with a description of why.
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 11/35] target/i386: access cpu->interrupt_request with atomics
2018-09-18 21:04 ` Richard Henderson
@ 2018-09-19 15:02 ` Emilio G. Cota
0 siblings, 0 replies; 18+ messages in thread
From: Emilio G. Cota @ 2018-09-19 15:02 UTC (permalink / raw)
To: Richard Henderson
Cc: Paolo Bonzini, Marcelo Tosatti, qemu-devel, kvm, Eduardo Habkost
On Tue, Sep 18, 2018 at 14:04:35 -0700, Richard Henderson wrote:
> On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> > cpu_reset(cs);
> > - cs->interrupt_request = sipi;
> > + atomic_mb_set(&cs->interrupt_request, sipi);
> > memcpy(&env->start_init_save, &save->start_init_save,
>
> Why does this need a memory barrier?
>
> Anyway, I think a bare mechanical conversion would be best
> for the first patch and then extra barriers added separately
> and with a description of why.
Almost no corresponding read has a barrier so it's hard to
justify this one. I'll drop it.
Thanks,
Emilio
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt
2018-09-17 16:30 [PATCH 00/35] exec: drop BQL from interrupt handling Emilio G. Cota
2018-09-17 16:30 ` [PATCH 02/35] target/i386: use cpu_reset_interrupt Emilio G. Cota
2018-09-17 16:30 ` [PATCH 11/35] target/i386: access cpu->interrupt_request with atomics Emilio G. Cota
@ 2018-09-17 16:31 ` Emilio G. Cota
2018-09-18 21:12 ` Richard Henderson
2018-09-19 21:19 ` Philippe Mathieu-Daudé
2018-09-17 16:31 ` [PATCH 34/35] exec: push BQL down to cpu->do_interrupt Emilio G. Cota
` (3 subsequent siblings)
6 siblings, 2 replies; 18+ messages in thread
From: Emilio G. Cota @ 2018-09-17 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Eduardo Habkost, kvm,
Richard Henderson
From: Paolo Bonzini <pbonzini@redhat.com>
It's not needed anymore.
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
target/i386/kvm.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index e40c8d5461..41c4830be8 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -3007,9 +3007,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
/* Inject NMI */
if (interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
if (interrupt_request & CPU_INTERRUPT_NMI) {
- qemu_mutex_lock_iothread();
cpu_reset_interrupt(cpu, CPU_INTERRUPT_NMI);
- qemu_mutex_unlock_iothread();
DPRINTF("injected NMI\n");
ret = kvm_vcpu_ioctl(cpu, KVM_NMI);
if (ret < 0) {
@@ -3018,9 +3016,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
}
}
if (atomic_read(&cpu->interrupt_request) & CPU_INTERRUPT_SMI) {
- qemu_mutex_lock_iothread();
cpu_reset_interrupt(cpu, CPU_INTERRUPT_SMI);
- qemu_mutex_unlock_iothread();
DPRINTF("injected SMI\n");
ret = kvm_vcpu_ioctl(cpu, KVM_SMI);
if (ret < 0) {
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt
2018-09-17 16:31 ` [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt Emilio G. Cota
@ 2018-09-18 21:12 ` Richard Henderson
2018-09-19 21:19 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2018-09-18 21:12 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Eduardo Habkost, kvm
On 9/17/18 9:31 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> It's not needed anymore.
>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
> target/i386/kvm.c | 4 ----
> 1 file changed, 4 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt
2018-09-17 16:31 ` [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt Emilio G. Cota
2018-09-18 21:12 ` Richard Henderson
@ 2018-09-19 21:19 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-09-19 21:19 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Eduardo Habkost, kvm,
Richard Henderson
On 9/17/18 6:31 PM, Emilio G. Cota wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> It's not needed anymore.
>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/i386/kvm.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index e40c8d5461..41c4830be8 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -3007,9 +3007,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
> /* Inject NMI */
> if (interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
> if (interrupt_request & CPU_INTERRUPT_NMI) {
> - qemu_mutex_lock_iothread();
> cpu_reset_interrupt(cpu, CPU_INTERRUPT_NMI);
> - qemu_mutex_unlock_iothread();
> DPRINTF("injected NMI\n");
> ret = kvm_vcpu_ioctl(cpu, KVM_NMI);
> if (ret < 0) {
> @@ -3018,9 +3016,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
> }
> }
> if (atomic_read(&cpu->interrupt_request) & CPU_INTERRUPT_SMI) {
> - qemu_mutex_lock_iothread();
> cpu_reset_interrupt(cpu, CPU_INTERRUPT_SMI);
> - qemu_mutex_unlock_iothread();
> DPRINTF("injected SMI\n");
> ret = kvm_vcpu_ioctl(cpu, KVM_SMI);
> if (ret < 0) {
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 34/35] exec: push BQL down to cpu->do_interrupt
2018-09-17 16:30 [PATCH 00/35] exec: drop BQL from interrupt handling Emilio G. Cota
` (2 preceding siblings ...)
2018-09-17 16:31 ` [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt Emilio G. Cota
@ 2018-09-17 16:31 ` Emilio G. Cota
2018-09-18 4:11 ` David Gibson
` (2 more replies)
2018-09-17 16:31 ` [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt Emilio G. Cota
` (2 subsequent siblings)
6 siblings, 3 replies; 18+ messages in thread
From: Emilio G. Cota @ 2018-09-17 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Cornelia Huck, kvm, David Hildenbrand, James Hogan,
Anthony Green, Mark Cave-Ayland, Edgar E. Iglesias, Guan Xuetao,
Marek Vasut, Alexander Graf, Christian Borntraeger,
Richard Henderson, Artyom Tarasenko, Eduardo Habkost, qemu-s390x,
qemu-arm, Stafford Horne, David Gibson, Chris Wulff,
Peter Crosthwaite, Marcelo Tosatti, Laurent Vivier, Michael Walle,
q
From: Paolo Bonzini <pbonzini@redhat.com>
cpu->do_interrupt can now be called with BQL held (from
cpu->cpu_exec_interrupt) or without (from cpu_handle_exception).
Only a few targets rely on global device state in cc->do_interrupt;
add checks to those targets to acquire the BQL if not already held.
Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Anthony Green <green@moxielogic.com>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Chris Wulff <crwulff@gmail.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: David Hildenbrand <david@redhat.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: James Hogan <jhogan@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Michael Walle <michael@walle.cc>
Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: qemu-ppc@nongnu.org
Cc: qemu-s390x@nongnu.org
Cc: Richard Henderson <rth@twiddle.net>
Cc: Stafford Horne <shorne@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
accel/tcg/cpu-exec.c | 2 --
target/arm/helper.c | 28 ++++++++++++++++++++++++++--
target/ppc/excp_helper.c | 8 +++++++-
target/s390x/excp_helper.c | 14 +++++++++++++-
target/sh4/helper.c | 14 +++++++++++++-
target/xtensa/helper.c | 16 ++++++++++++++--
6 files changed, 73 insertions(+), 9 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 2383763f9b..b649e3d772 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -497,9 +497,7 @@ 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 */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 22dbc42305..548278da14 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7644,7 +7644,8 @@ gen_invep:
return false;
}
-void arm_v7m_cpu_do_interrupt(CPUState *cs)
+/* call with the BQL held */
+static void arm_v7m_cpu_do_interrupt_locked(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -7828,6 +7829,17 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
v7m_exception_taken(cpu, lr, false, ignore_stackfaults);
}
+void arm_v7m_cpu_do_interrupt(CPUState *cs)
+{
+ if (qemu_mutex_iothread_locked()) {
+ arm_v7m_cpu_do_interrupt_locked(cs);
+ } else {
+ qemu_mutex_lock_iothread();
+ arm_v7m_cpu_do_interrupt_locked(cs);
+ qemu_mutex_unlock_iothread();
+ }
+}
+
/* Function used to synchronize QEMU's AArch64 register set with AArch32
* register set. This is necessary when switching between AArch32 and AArch64
* execution state.
@@ -8482,8 +8494,9 @@ static inline bool check_for_semihosting(CPUState *cs)
* Do any appropriate logging, handle PSCI calls, and then hand off
* to the AArch64-entry or AArch32-entry function depending on the
* target exception level's register width.
+ * Call with the BQL held.
*/
-void arm_cpu_do_interrupt(CPUState *cs)
+static void arm_cpu_do_interrupt_locked(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -8534,6 +8547,17 @@ void arm_cpu_do_interrupt(CPUState *cs)
}
}
+void arm_cpu_do_interrupt(CPUState *cs)
+{
+ if (qemu_mutex_iothread_locked()) {
+ arm_cpu_do_interrupt_locked(cs);
+ } else {
+ qemu_mutex_lock_iothread();
+ arm_cpu_do_interrupt_locked(cs);
+ qemu_mutex_unlock_iothread();
+ }
+}
+
/* Return the exception level which controls this address translation regime */
static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
{
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 70ac10e23b..8b2cc48cad 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -742,7 +742,13 @@ void ppc_cpu_do_interrupt(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = &cpu->env;
- powerpc_excp(cpu, env->excp_model, cs->exception_index);
+ if (qemu_mutex_iothread_locked()) {
+ powerpc_excp(cpu, env->excp_model, cs->exception_index);
+ } else {
+ qemu_mutex_lock_iothread();
+ powerpc_excp(cpu, env->excp_model, cs->exception_index);
+ qemu_mutex_unlock_iothread();
+ }
}
static void ppc_hw_interrupt(CPUPPCState *env)
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index f2b92d7cbc..931c0103c8 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -378,7 +378,8 @@ static void do_mchk_interrupt(CPUS390XState *env)
load_psw(env, mask, addr);
}
-void s390_cpu_do_interrupt(CPUState *cs)
+/* call with the BQL held */
+static void s390_cpu_do_interrupt_locked(CPUState *cs)
{
QEMUS390FLICState *flic = QEMU_S390_FLIC(s390_get_flic());
S390CPU *cpu = S390_CPU(cs);
@@ -457,6 +458,17 @@ try_deliver:
}
}
+void s390_cpu_do_interrupt(CPUState *cs)
+{
+ if (qemu_mutex_iothread_locked()) {
+ s390_cpu_do_interrupt_locked(cs);
+ } else {
+ qemu_mutex_lock_iothread();
+ s390_cpu_do_interrupt_locked(cs);
+ qemu_mutex_unlock_iothread();
+ }
+}
+
bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
if (interrupt_request & CPU_INTERRUPT_HARD) {
diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index c699b8c0a1..6c508cd006 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -79,7 +79,8 @@ int cpu_sh4_is_cached(CPUSH4State * env, target_ulong addr)
#define MMU_DADDR_ERROR_READ (-12)
#define MMU_DADDR_ERROR_WRITE (-13)
-void superh_cpu_do_interrupt(CPUState *cs)
+/* call with the BQL held */
+static void superh_cpu_do_interrupt_locked(CPUState *cs)
{
SuperHCPU *cpu = SUPERH_CPU(cs);
CPUSH4State *env = &cpu->env;
@@ -211,6 +212,17 @@ void superh_cpu_do_interrupt(CPUState *cs)
}
}
+void superh_cpu_do_interrupt(CPUState *cs)
+{
+ if (qemu_mutex_iothread_locked()) {
+ superh_cpu_do_interrupt_locked(cs);
+ } else {
+ qemu_mutex_lock_iothread();
+ superh_cpu_do_interrupt_locked(cs);
+ qemu_mutex_unlock_iothread();
+ }
+}
+
static void update_itlb_use(CPUSH4State * env, int itlbnb)
{
uint8_t or_mask = 0, and_mask = (uint8_t) - 1;
diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
index c9a6132700..ecafecdd3f 100644
--- a/target/xtensa/helper.c
+++ b/target/xtensa/helper.c
@@ -26,6 +26,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "qemu/units.h"
#include "cpu.h"
#include "exec/exec-all.h"
@@ -251,8 +252,8 @@ static void handle_interrupt(CPUXtensaState *env)
}
}
-/* Called from cpu_handle_interrupt with BQL held */
-void xtensa_cpu_do_interrupt(CPUState *cs)
+/* Call with the BQL held */
+static void xtensa_cpu_do_interrupt_locked(CPUState *cs)
{
XtensaCPU *cpu = XTENSA_CPU(cs);
CPUXtensaState *env = &cpu->env;
@@ -305,6 +306,17 @@ void xtensa_cpu_do_interrupt(CPUState *cs)
}
check_interrupts(env);
}
+
+void xtensa_cpu_do_interrupt(CPUState *cs)
+{
+ if (qemu_mutex_iothread_locked()) {
+ xtensa_cpu_do_interrupt_locked(cs);
+ } else {
+ qemu_mutex_lock_iothread();
+ xtensa_cpu_do_interrupt_locked(cs);
+ qemu_mutex_unlock_iothread();
+ }
+}
#else
void xtensa_cpu_do_interrupt(CPUState *cs)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 34/35] exec: push BQL down to cpu->do_interrupt
2018-09-17 16:31 ` [PATCH 34/35] exec: push BQL down to cpu->do_interrupt Emilio G. Cota
@ 2018-09-18 4:11 ` David Gibson
2018-09-18 7:12 ` David Hildenbrand
2018-09-19 16:38 ` Cornelia Huck
2 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2018-09-18 4:11 UTC (permalink / raw)
To: Emilio G. Cota
Cc: Peter Maydell, Cornelia Huck, kvm, David Hildenbrand, James Hogan,
Anthony Green, Mark Cave-Ayland, qemu-devel, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Alexander Graf, Christian Borntraeger,
Artyom Tarasenko, Eduardo Habkost, qemu-s390x, qemu-arm,
Stafford Horne, Richard Henderson, Chris Wulff, Peter Crosthwaite,
Marcelo Tosatti, Laurent Vivier, Michael Walle, qemu-ppc, Al
[-- Attachment #1: Type: text/plain, Size: 9121 bytes --]
On Mon, Sep 17, 2018 at 12:31:02PM -0400, Emilio G. Cota wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> cpu->do_interrupt can now be called with BQL held (from
> cpu->cpu_exec_interrupt) or without (from cpu_handle_exception).
>
> Only a few targets rely on global device state in cc->do_interrupt;
> add checks to those targets to acquire the BQL if not already held.
>
> Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Anthony Green <green@moxielogic.com>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Chris Wulff <crwulff@gmail.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
ppc parts
Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> accel/tcg/cpu-exec.c | 2 --
> target/arm/helper.c | 28 ++++++++++++++++++++++++++--
> target/ppc/excp_helper.c | 8 +++++++-
> target/s390x/excp_helper.c | 14 +++++++++++++-
> target/sh4/helper.c | 14 +++++++++++++-
> target/xtensa/helper.c | 16 ++++++++++++++--
> 6 files changed, 73 insertions(+), 9 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 2383763f9b..b649e3d772 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -497,9 +497,7 @@ 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 */
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 22dbc42305..548278da14 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -7644,7 +7644,8 @@ gen_invep:
> return false;
> }
>
> -void arm_v7m_cpu_do_interrupt(CPUState *cs)
> +/* call with the BQL held */
> +static void arm_v7m_cpu_do_interrupt_locked(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -7828,6 +7829,17 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> v7m_exception_taken(cpu, lr, false, ignore_stackfaults);
> }
>
> +void arm_v7m_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + arm_v7m_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + arm_v7m_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> +
> /* Function used to synchronize QEMU's AArch64 register set with AArch32
> * register set. This is necessary when switching between AArch32 and AArch64
> * execution state.
> @@ -8482,8 +8494,9 @@ static inline bool check_for_semihosting(CPUState *cs)
> * Do any appropriate logging, handle PSCI calls, and then hand off
> * to the AArch64-entry or AArch32-entry function depending on the
> * target exception level's register width.
> + * Call with the BQL held.
> */
> -void arm_cpu_do_interrupt(CPUState *cs)
> +static void arm_cpu_do_interrupt_locked(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -8534,6 +8547,17 @@ void arm_cpu_do_interrupt(CPUState *cs)
> }
> }
>
> +void arm_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + arm_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + arm_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> +
> /* Return the exception level which controls this address translation regime */
> static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
> {
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 70ac10e23b..8b2cc48cad 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -742,7 +742,13 @@ void ppc_cpu_do_interrupt(CPUState *cs)
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
>
> - powerpc_excp(cpu, env->excp_model, cs->exception_index);
> + if (qemu_mutex_iothread_locked()) {
> + powerpc_excp(cpu, env->excp_model, cs->exception_index);
> + } else {
> + qemu_mutex_lock_iothread();
> + powerpc_excp(cpu, env->excp_model, cs->exception_index);
> + qemu_mutex_unlock_iothread();
> + }
> }
>
> static void ppc_hw_interrupt(CPUPPCState *env)
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index f2b92d7cbc..931c0103c8 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -378,7 +378,8 @@ static void do_mchk_interrupt(CPUS390XState *env)
> load_psw(env, mask, addr);
> }
>
> -void s390_cpu_do_interrupt(CPUState *cs)
> +/* call with the BQL held */
> +static void s390_cpu_do_interrupt_locked(CPUState *cs)
> {
> QEMUS390FLICState *flic = QEMU_S390_FLIC(s390_get_flic());
> S390CPU *cpu = S390_CPU(cs);
> @@ -457,6 +458,17 @@ try_deliver:
> }
> }
>
> +void s390_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + s390_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + s390_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> +
> bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> {
> if (interrupt_request & CPU_INTERRUPT_HARD) {
> diff --git a/target/sh4/helper.c b/target/sh4/helper.c
> index c699b8c0a1..6c508cd006 100644
> --- a/target/sh4/helper.c
> +++ b/target/sh4/helper.c
> @@ -79,7 +79,8 @@ int cpu_sh4_is_cached(CPUSH4State * env, target_ulong addr)
> #define MMU_DADDR_ERROR_READ (-12)
> #define MMU_DADDR_ERROR_WRITE (-13)
>
> -void superh_cpu_do_interrupt(CPUState *cs)
> +/* call with the BQL held */
> +static void superh_cpu_do_interrupt_locked(CPUState *cs)
> {
> SuperHCPU *cpu = SUPERH_CPU(cs);
> CPUSH4State *env = &cpu->env;
> @@ -211,6 +212,17 @@ void superh_cpu_do_interrupt(CPUState *cs)
> }
> }
>
> +void superh_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + superh_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + superh_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> +
> static void update_itlb_use(CPUSH4State * env, int itlbnb)
> {
> uint8_t or_mask = 0, and_mask = (uint8_t) - 1;
> diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
> index c9a6132700..ecafecdd3f 100644
> --- a/target/xtensa/helper.c
> +++ b/target/xtensa/helper.c
> @@ -26,6 +26,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> #include "qemu/units.h"
> #include "cpu.h"
> #include "exec/exec-all.h"
> @@ -251,8 +252,8 @@ static void handle_interrupt(CPUXtensaState *env)
> }
> }
>
> -/* Called from cpu_handle_interrupt with BQL held */
> -void xtensa_cpu_do_interrupt(CPUState *cs)
> +/* Call with the BQL held */
> +static void xtensa_cpu_do_interrupt_locked(CPUState *cs)
> {
> XtensaCPU *cpu = XTENSA_CPU(cs);
> CPUXtensaState *env = &cpu->env;
> @@ -305,6 +306,17 @@ void xtensa_cpu_do_interrupt(CPUState *cs)
> }
> check_interrupts(env);
> }
> +
> +void xtensa_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + xtensa_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + xtensa_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> #else
> void xtensa_cpu_do_interrupt(CPUState *cs)
> {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 34/35] exec: push BQL down to cpu->do_interrupt
2018-09-17 16:31 ` [PATCH 34/35] exec: push BQL down to cpu->do_interrupt Emilio G. Cota
2018-09-18 4:11 ` David Gibson
@ 2018-09-18 7:12 ` David Hildenbrand
2018-09-19 16:38 ` Cornelia Huck
2 siblings, 0 replies; 18+ messages in thread
From: David Hildenbrand @ 2018-09-18 7:12 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Peter Maydell, Cornelia Huck, kvm, James Hogan, Anthony Green,
Mark Cave-Ayland, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Alexander Graf, Christian Borntraeger, Richard Henderson,
Artyom Tarasenko, Eduardo Habkost, qemu-s390x, qemu-arm,
Stafford Horne, David Gibson, Chris Wulff, Peter Crosthwaite,
Marcelo Tosatti, Laurent Vivier, Michael Walle, qemu-ppc,
Aleksandar
Am 17.09.18 um 18:31 schrieb Emilio G. Cota:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> cpu->do_interrupt can now be called with BQL held (from
> cpu->cpu_exec_interrupt) or without (from cpu_handle_exception).
>
> Only a few targets rely on global device state in cc->do_interrupt;
> add checks to those targets to acquire the BQL if not already held.
>
> Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Anthony Green <green@moxielogic.com>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Chris Wulff <crwulff@gmail.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
> accel/tcg/cpu-exec.c | 2 --
> target/arm/helper.c | 28 ++++++++++++++++++++++++++--
> target/ppc/excp_helper.c | 8 +++++++-
> target/s390x/excp_helper.c | 14 +++++++++++++-
> target/sh4/helper.c | 14 +++++++++++++-
> target/xtensa/helper.c | 16 ++++++++++++++--
> 6 files changed, 73 insertions(+), 9 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 2383763f9b..b649e3d772 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -497,9 +497,7 @@ 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 */
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 22dbc42305..548278da14 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -7644,7 +7644,8 @@ gen_invep:
> return false;
> }
>
> -void arm_v7m_cpu_do_interrupt(CPUState *cs)
> +/* call with the BQL held */
> +static void arm_v7m_cpu_do_interrupt_locked(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -7828,6 +7829,17 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> v7m_exception_taken(cpu, lr, false, ignore_stackfaults);
> }
>
> +void arm_v7m_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + arm_v7m_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + arm_v7m_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> +
> /* Function used to synchronize QEMU's AArch64 register set with AArch32
> * register set. This is necessary when switching between AArch32 and AArch64
> * execution state.
> @@ -8482,8 +8494,9 @@ static inline bool check_for_semihosting(CPUState *cs)
> * Do any appropriate logging, handle PSCI calls, and then hand off
> * to the AArch64-entry or AArch32-entry function depending on the
> * target exception level's register width.
> + * Call with the BQL held.
> */
> -void arm_cpu_do_interrupt(CPUState *cs)
> +static void arm_cpu_do_interrupt_locked(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -8534,6 +8547,17 @@ void arm_cpu_do_interrupt(CPUState *cs)
> }
> }
>
> +void arm_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + arm_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + arm_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> +
> /* Return the exception level which controls this address translation regime */
> static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
> {
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 70ac10e23b..8b2cc48cad 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -742,7 +742,13 @@ void ppc_cpu_do_interrupt(CPUState *cs)
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
>
> - powerpc_excp(cpu, env->excp_model, cs->exception_index);
> + if (qemu_mutex_iothread_locked()) {
> + powerpc_excp(cpu, env->excp_model, cs->exception_index);
> + } else {
> + qemu_mutex_lock_iothread();
> + powerpc_excp(cpu, env->excp_model, cs->exception_index);
> + qemu_mutex_unlock_iothread();
> + }
> }
>
> static void ppc_hw_interrupt(CPUPPCState *env)
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index f2b92d7cbc..931c0103c8 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -378,7 +378,8 @@ static void do_mchk_interrupt(CPUS390XState *env)
> load_psw(env, mask, addr);
> }
>
> -void s390_cpu_do_interrupt(CPUState *cs)
> +/* call with the BQL held */
> +static void s390_cpu_do_interrupt_locked(CPUState *cs)
> {
> QEMUS390FLICState *flic = QEMU_S390_FLIC(s390_get_flic());
> S390CPU *cpu = S390_CPU(cs);
> @@ -457,6 +458,17 @@ try_deliver:
> }
> }
>
> +void s390_cpu_do_interrupt(CPUState *cs)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + s390_cpu_do_interrupt_locked(cs);
> + } else {
> + qemu_mutex_lock_iothread();
> + s390_cpu_do_interrupt_locked(cs);
> + qemu_mutex_unlock_iothread();
> + }
> +}
> +
Yes, due to floating interrupts we need the iothread lock. This change
looks sane to me from an s390x perspective:
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 34/35] exec: push BQL down to cpu->do_interrupt
2018-09-17 16:31 ` [PATCH 34/35] exec: push BQL down to cpu->do_interrupt Emilio G. Cota
2018-09-18 4:11 ` David Gibson
2018-09-18 7:12 ` David Hildenbrand
@ 2018-09-19 16:38 ` Cornelia Huck
2 siblings, 0 replies; 18+ messages in thread
From: Cornelia Huck @ 2018-09-19 16:38 UTC (permalink / raw)
To: Emilio G. Cota
Cc: Peter Maydell, Chris Wulff, kvm, David Hildenbrand, James Hogan,
Anthony Green, Mark Cave-Ayland, qemu-devel, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Alexander Graf, Christian Borntraeger,
Richard Henderson, Artyom Tarasenko, Eduardo Habkost, qemu-s390x,
qemu-arm, Stafford Horne, David Gibson, Peter Crosthwaite,
Marcelo Tosatti, Laurent Vivier, Michael Walle, qemu-ppc
On Mon, 17 Sep 2018 12:31:02 -0400
"Emilio G. Cota" <cota@braap.org> wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> cpu->do_interrupt can now be called with BQL held (from
> cpu->cpu_exec_interrupt) or without (from cpu_handle_exception).
>
> Only a few targets rely on global device state in cc->do_interrupt;
> add checks to those targets to acquire the BQL if not already held.
>
> Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Anthony Green <green@moxielogic.com>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Chris Wulff <crwulff@gmail.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
> accel/tcg/cpu-exec.c | 2 --
> target/arm/helper.c | 28 ++++++++++++++++++++++++++--
> target/ppc/excp_helper.c | 8 +++++++-
> target/s390x/excp_helper.c | 14 +++++++++++++-
> target/sh4/helper.c | 14 +++++++++++++-
> target/xtensa/helper.c | 16 ++++++++++++++--
> 6 files changed, 73 insertions(+), 9 deletions(-)
s390x parts:
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt
2018-09-17 16:30 [PATCH 00/35] exec: drop BQL from interrupt handling Emilio G. Cota
` (3 preceding siblings ...)
2018-09-17 16:31 ` [PATCH 34/35] exec: push BQL down to cpu->do_interrupt Emilio G. Cota
@ 2018-09-17 16:31 ` Emilio G. Cota
2018-09-18 4:12 ` David Gibson
2018-09-18 7:48 ` David Hildenbrand
2018-09-18 9:51 ` [PATCH 00/35] exec: drop BQL from interrupt handling David Hildenbrand
2018-09-20 20:05 ` Mark Cave-Ayland
6 siblings, 2 replies; 18+ messages in thread
From: Emilio G. Cota @ 2018-09-17 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Cornelia Huck, kvm, David Hildenbrand, James Hogan,
Anthony Green, Mark Cave-Ayland, Edgar E. Iglesias, Guan Xuetao,
Marek Vasut, Alexander Graf, Christian Borntraeger,
Richard Henderson, Artyom Tarasenko, Eduardo Habkost, qemu-s390x,
qemu-arm, Stafford Horne, David Gibson, Chris Wulff,
Peter Crosthwaite, Marcelo Tosatti, Laurent Vivier, Michael Walle,
q
From: Paolo Bonzini <pbonzini@redhat.com>
Most interrupt requests do not need to take the BQL, and in fact
most architectures do not need it at all. Push the BQL acquisition
down to target code.
Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Anthony Green <green@moxielogic.com>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Chris Wulff <crwulff@gmail.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: David Hildenbrand <david@redhat.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: James Hogan <jhogan@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Michael Walle <michael@walle.cc>
Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: qemu-ppc@nongnu.org
Cc: qemu-s390x@nongnu.org
Cc: Richard Henderson <rth@twiddle.net>
Cc: Stafford Horne <shorne@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
docs/devel/multi-thread-tcg.txt | 7 +++++--
accel/tcg/cpu-exec.c | 9 +--------
target/arm/cpu.c | 15 ++++++++++++++-
target/i386/seg_helper.c | 3 +++
target/ppc/excp_helper.c | 2 ++
target/s390x/excp_helper.c | 3 +++
6 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/docs/devel/multi-thread-tcg.txt b/docs/devel/multi-thread-tcg.txt
index 782bebc28b..422de4736b 100644
--- a/docs/devel/multi-thread-tcg.txt
+++ b/docs/devel/multi-thread-tcg.txt
@@ -231,8 +231,11 @@ BQL. Currently ARM targets serialise all ARM_CP_IO register accesses
and also defer the reset/startup of vCPUs to the vCPU context by way
of async_run_on_cpu().
-Updates to interrupt state are also protected by the BQL as they can
-often be cross vCPU.
+The CPUClass callbacks cpu_exec_interrupt and do_interrupt are invoked
+without BQL protection. Accesses to the interrupt controller from
+the vCPU thread, for example while processing CPU_INTERRUPT_HARD, must
+either call qemu_mutex_lock_iothread/qemu_mutex_unlock_iothread or use
+a separate mutex.
Memory Consistency
==================
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index b649e3d772..f5e08e79d1 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -524,7 +524,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
if (unlikely(atomic_read(&cpu->interrupt_request))) {
int interrupt_request;
- qemu_mutex_lock_iothread();
+
interrupt_request = atomic_read(&cpu->interrupt_request);
if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
/* Mask out external interrupts for this step. */
@@ -533,7 +533,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
if (interrupt_request & CPU_INTERRUPT_DEBUG) {
cpu_reset_interrupt(cpu, CPU_INTERRUPT_DEBUG);
cpu->exception_index = EXCP_DEBUG;
- qemu_mutex_unlock_iothread();
return true;
}
if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
@@ -543,7 +542,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
cpu->halted = 1;
cpu->exception_index = EXCP_HLT;
- qemu_mutex_unlock_iothread();
return true;
}
#if defined(TARGET_I386)
@@ -554,14 +552,12 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
do_cpu_init(x86_cpu);
cpu->exception_index = EXCP_HALTED;
- qemu_mutex_unlock_iothread();
return true;
}
#else
else if (interrupt_request & CPU_INTERRUPT_RESET) {
replay_interrupt();
cpu_reset(cpu);
- qemu_mutex_unlock_iothread();
return true;
}
#endif
@@ -585,9 +581,6 @@ static inline bool 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();
}
/* Finally, check if we need to exit to the main loop. */
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index e2c492efdf..246ea13d8f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -347,7 +347,8 @@ static void arm_cpu_reset(CPUState *s)
hw_watchpoint_update_all(cpu);
}
-bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
+/* call with the BQL held */
+static bool arm_cpu_exec_interrupt_locked(CPUState *cs, int interrupt_request)
{
CPUClass *cc = CPU_GET_CLASS(cs);
CPUARMState *env = cs->env_ptr;
@@ -401,6 +402,16 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
return ret;
}
+bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
+{
+ bool ret;
+
+ qemu_mutex_lock_iothread();
+ ret = arm_cpu_exec_interrupt_locked(cs, interrupt_request);
+ qemu_mutex_unlock_iothread();
+ return ret;
+}
+
#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
@@ -409,6 +420,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
CPUARMState *env = &cpu->env;
bool ret = false;
+ qemu_mutex_lock_iothread();
/* ARMv7-M interrupt masking works differently than -A or -R.
* There is no FIQ/IRQ distinction. Instead of I and F bits
* masking FIQ and IRQ interrupts, an exception is taken only
@@ -422,6 +434,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
cc->do_interrupt(cs);
ret = true;
}
+ qemu_mutex_unlock_iothread();
return ret;
}
#endif
diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
index 0dd85329db..2fdfbd3c37 100644
--- a/target/i386/seg_helper.c
+++ b/target/i386/seg_helper.c
@@ -19,6 +19,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "qemu/log.h"
#include "exec/helper-proto.h"
@@ -1324,7 +1325,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
#if !defined(CONFIG_USER_ONLY)
if (interrupt_request & CPU_INTERRUPT_POLL) {
cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL);
+ qemu_mutex_lock_iothread();
apic_poll_irq(cpu->apic_state);
+ qemu_mutex_unlock_iothread();
/* Don't process multiple interrupt requests in a single call.
This is required to make icount-driven execution deterministic. */
return true;
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 8b2cc48cad..57acba2a80 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -885,10 +885,12 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
CPUPPCState *env = &cpu->env;
if (interrupt_request & CPU_INTERRUPT_HARD) {
+ qemu_mutex_lock_iothread();
ppc_hw_interrupt(env);
if (env->pending_interrupts == 0) {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
+ qemu_mutex_unlock_iothread();
return true;
}
return false;
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 931c0103c8..f2a93abf01 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -480,10 +480,13 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
the parent EXECUTE insn. */
return false;
}
+ qemu_mutex_lock_iothread();
if (s390_cpu_has_int(cpu)) {
s390_cpu_do_interrupt(cs);
+ qemu_mutex_unlock_iothread();
return true;
}
+ qemu_mutex_unlock_iothread();
if (env->psw.mask & PSW_MASK_WAIT) {
/* Woken up because of a floating interrupt but it has already
* been delivered. Go back to sleep. */
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt
2018-09-17 16:31 ` [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt Emilio G. Cota
@ 2018-09-18 4:12 ` David Gibson
2018-09-18 7:48 ` David Hildenbrand
1 sibling, 0 replies; 18+ messages in thread
From: David Gibson @ 2018-09-18 4:12 UTC (permalink / raw)
To: Emilio G. Cota
Cc: Peter Maydell, Cornelia Huck, kvm, David Hildenbrand, James Hogan,
Anthony Green, Mark Cave-Ayland, qemu-devel, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Alexander Graf, Christian Borntraeger,
Artyom Tarasenko, Eduardo Habkost, qemu-s390x, qemu-arm,
Stafford Horne, Richard Henderson, Chris Wulff, Peter Crosthwaite,
Marcelo Tosatti, Laurent Vivier, Michael Walle, qemu-ppc, Al
[-- Attachment #1: Type: text/plain, Size: 9405 bytes --]
On Mon, Sep 17, 2018 at 12:31:03PM -0400, Emilio G. Cota wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> Most interrupt requests do not need to take the BQL, and in fact
> most architectures do not need it at all. Push the BQL acquisition
> down to target code.
>
> Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Anthony Green <green@moxielogic.com>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Chris Wulff <crwulff@gmail.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
ppc parts
Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> docs/devel/multi-thread-tcg.txt | 7 +++++--
> accel/tcg/cpu-exec.c | 9 +--------
> target/arm/cpu.c | 15 ++++++++++++++-
> target/i386/seg_helper.c | 3 +++
> target/ppc/excp_helper.c | 2 ++
> target/s390x/excp_helper.c | 3 +++
> 6 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/docs/devel/multi-thread-tcg.txt b/docs/devel/multi-thread-tcg.txt
> index 782bebc28b..422de4736b 100644
> --- a/docs/devel/multi-thread-tcg.txt
> +++ b/docs/devel/multi-thread-tcg.txt
> @@ -231,8 +231,11 @@ BQL. Currently ARM targets serialise all ARM_CP_IO register accesses
> and also defer the reset/startup of vCPUs to the vCPU context by way
> of async_run_on_cpu().
>
> -Updates to interrupt state are also protected by the BQL as they can
> -often be cross vCPU.
> +The CPUClass callbacks cpu_exec_interrupt and do_interrupt are invoked
> +without BQL protection. Accesses to the interrupt controller from
> +the vCPU thread, for example while processing CPU_INTERRUPT_HARD, must
> +either call qemu_mutex_lock_iothread/qemu_mutex_unlock_iothread or use
> +a separate mutex.
>
> Memory Consistency
> ==================
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index b649e3d772..f5e08e79d1 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -524,7 +524,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>
> if (unlikely(atomic_read(&cpu->interrupt_request))) {
> int interrupt_request;
> - qemu_mutex_lock_iothread();
> +
> interrupt_request = atomic_read(&cpu->interrupt_request);
> if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
> /* Mask out external interrupts for this step. */
> @@ -533,7 +533,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> if (interrupt_request & CPU_INTERRUPT_DEBUG) {
> cpu_reset_interrupt(cpu, CPU_INTERRUPT_DEBUG);
> cpu->exception_index = EXCP_DEBUG;
> - qemu_mutex_unlock_iothread();
> return true;
> }
> if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
> @@ -543,7 +542,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
> cpu->halted = 1;
> cpu->exception_index = EXCP_HLT;
> - qemu_mutex_unlock_iothread();
> return true;
> }
> #if defined(TARGET_I386)
> @@ -554,14 +552,12 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
> do_cpu_init(x86_cpu);
> cpu->exception_index = EXCP_HALTED;
> - qemu_mutex_unlock_iothread();
> return true;
> }
> #else
> else if (interrupt_request & CPU_INTERRUPT_RESET) {
> replay_interrupt();
> cpu_reset(cpu);
> - qemu_mutex_unlock_iothread();
> return true;
> }
> #endif
> @@ -585,9 +581,6 @@ static inline bool 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();
> }
>
> /* Finally, check if we need to exit to the main loop. */
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index e2c492efdf..246ea13d8f 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -347,7 +347,8 @@ static void arm_cpu_reset(CPUState *s)
> hw_watchpoint_update_all(cpu);
> }
>
> -bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> +/* call with the BQL held */
> +static bool arm_cpu_exec_interrupt_locked(CPUState *cs, int interrupt_request)
> {
> CPUClass *cc = CPU_GET_CLASS(cs);
> CPUARMState *env = cs->env_ptr;
> @@ -401,6 +402,16 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> return ret;
> }
>
> +bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> +{
> + bool ret;
> +
> + qemu_mutex_lock_iothread();
> + ret = arm_cpu_exec_interrupt_locked(cs, interrupt_request);
> + qemu_mutex_unlock_iothread();
> + return ret;
> +}
> +
> #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
> static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> {
> @@ -409,6 +420,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> CPUARMState *env = &cpu->env;
> bool ret = false;
>
> + qemu_mutex_lock_iothread();
> /* ARMv7-M interrupt masking works differently than -A or -R.
> * There is no FIQ/IRQ distinction. Instead of I and F bits
> * masking FIQ and IRQ interrupts, an exception is taken only
> @@ -422,6 +434,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> cc->do_interrupt(cs);
> ret = true;
> }
> + qemu_mutex_unlock_iothread();
> return ret;
> }
> #endif
> diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
> index 0dd85329db..2fdfbd3c37 100644
> --- a/target/i386/seg_helper.c
> +++ b/target/i386/seg_helper.c
> @@ -19,6 +19,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> #include "cpu.h"
> #include "qemu/log.h"
> #include "exec/helper-proto.h"
> @@ -1324,7 +1325,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> #if !defined(CONFIG_USER_ONLY)
> if (interrupt_request & CPU_INTERRUPT_POLL) {
> cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL);
> + qemu_mutex_lock_iothread();
> apic_poll_irq(cpu->apic_state);
> + qemu_mutex_unlock_iothread();
> /* Don't process multiple interrupt requests in a single call.
> This is required to make icount-driven execution deterministic. */
> return true;
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 8b2cc48cad..57acba2a80 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -885,10 +885,12 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> CPUPPCState *env = &cpu->env;
>
> if (interrupt_request & CPU_INTERRUPT_HARD) {
> + qemu_mutex_lock_iothread();
> ppc_hw_interrupt(env);
> if (env->pending_interrupts == 0) {
> cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> + qemu_mutex_unlock_iothread();
> return true;
> }
> return false;
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index 931c0103c8..f2a93abf01 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -480,10 +480,13 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> the parent EXECUTE insn. */
> return false;
> }
> + qemu_mutex_lock_iothread();
> if (s390_cpu_has_int(cpu)) {
> s390_cpu_do_interrupt(cs);
> + qemu_mutex_unlock_iothread();
> return true;
> }
> + qemu_mutex_unlock_iothread();
> if (env->psw.mask & PSW_MASK_WAIT) {
> /* Woken up because of a floating interrupt but it has already
> * been delivered. Go back to sleep. */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt
2018-09-17 16:31 ` [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt Emilio G. Cota
2018-09-18 4:12 ` David Gibson
@ 2018-09-18 7:48 ` David Hildenbrand
1 sibling, 0 replies; 18+ messages in thread
From: David Hildenbrand @ 2018-09-18 7:48 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Peter Maydell, Cornelia Huck, kvm, James Hogan, Anthony Green,
Mark Cave-Ayland, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Alexander Graf, Christian Borntraeger, Richard Henderson,
Artyom Tarasenko, Eduardo Habkost, qemu-s390x, qemu-arm,
Stafford Horne, David Gibson, Chris Wulff, Peter Crosthwaite,
Marcelo Tosatti, Laurent Vivier, Michael Walle, qemu-ppc,
Aleksandar
> return false;
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index 931c0103c8..f2a93abf01 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -480,10 +480,13 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> the parent EXECUTE insn. */
> return false;
> }
> + qemu_mutex_lock_iothread();
> if (s390_cpu_has_int(cpu)) {
> s390_cpu_do_interrupt(cs);
You can directly use s390_cpu_do_interrupt_locked() here.
> + qemu_mutex_unlock_iothread();
> return true;
> }
> + qemu_mutex_unlock_iothread();
> if (env->psw.mask & PSW_MASK_WAIT) {
> /* Woken up because of a floating interrupt but it has already
> * been delivered. Go back to sleep. */
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/35] exec: drop BQL from interrupt handling
2018-09-17 16:30 [PATCH 00/35] exec: drop BQL from interrupt handling Emilio G. Cota
` (4 preceding siblings ...)
2018-09-17 16:31 ` [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt Emilio G. Cota
@ 2018-09-18 9:51 ` David Hildenbrand
2018-09-20 20:05 ` Mark Cave-Ayland
6 siblings, 0 replies; 18+ messages in thread
From: David Hildenbrand @ 2018-09-18 9:51 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Peter Maydell, Cornelia Huck, kvm, James Hogan, Anthony Green,
Mark Cave-Ayland, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Alexander Graf, Christian Borntraeger, Richard Henderson,
Artyom Tarasenko, Eduardo Habkost, qemu-s390x, qemu-arm,
Stafford Horne, David Gibson, Chris Wulff, Peter Crosthwaite,
Marcelo Tosatti, Laurent Vivier, Michael Walle, qemu-ppc,
Aleksandar
Am 17.09.18 um 18:30 schrieb Emilio G. Cota:
> This series comes originally from a series of patches that Paolo
> sent to me a long time ago. I have kept most of his S-o-b tags,
> but I have done the forward port of the patches to the current
> QEMU code base, so please blame all possible bugs on me, not him.
>
> The goal of this series is to push the BQL out of interrupt
> handling wherever possible. Many targets do not need the BQL
> to handle interrupts, so for those targets we get rid of the
> BQL in interrupt handling. In TCG mode, this by itself does
> not yield measurable performance gains. However, it is a
> necessary first step before tackling BQL contention when
> managing CPU states (i.e. exiting the exec loop always
> involves taking the BQL, which does not scale).
>
> The patches:
>
> - Patch 0 accesses icount_decr.u16 consistently with atomics.
> This is important because we use it across threads to signal
> that an interrupt is pending.
>
> - 2-28: convert direct callers of cpu->interrupt_request
> to use atomic accesses or CPU helper functions that use atomics.
> I have broken these up into individual patches to ease review.
> Note that in some cases I have added many atomic_reads, most
> of which are probably unnecessary. However, I'd leave this up
> to the maintainers, since I do not want to break things by
> doing a single atomic_read(&cpu->interrupt_request) at the
> beginning of a function and then miss subsequent updates to
> cpu->interrupt_request.
>
> - 29-31: drop the BQL requirement when handling cpu->interrupt_request
>
> - 32-33: drop now unnecessary BQL acquisitions in i386 and ppc
>
> - 34-35: push down the BQL acquisition to cpu->do_interrupt
> and cpu->cpu_exec_interrupt. Here I am Cc'ing everyone so that
> we can make sure I am not missing any target that requires
> the BQL on either do_interrupt or cpu_exec_interrupt.
>
> This series is checkpatch-clean, and can be fetched from:
> https://github.com/cota/qemu/tree/cpu-lock
>
> Note that the series applies on top of patches that are
> already on the qemu-devel list and will hopefully soon
> be upstream. This series begins after commit
> 4ece0cbd74 ("cpus: access .qemu_icount_bias with atomic64")
> in that tree.
>
> Thanks,
>
> Emilio
>
I just did a (very slow) make -j32 of QEMU on my 16 core notebook inside
a MTTCG s390x-softmmu guest with 32 VCPUs. I used the same test when
reworking floating interrupt support to make sure there are no races.
As I got no deadlock, I assume you series does not break anything for s390x.
Thanks!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 00/35] exec: drop BQL from interrupt handling
2018-09-17 16:30 [PATCH 00/35] exec: drop BQL from interrupt handling Emilio G. Cota
` (5 preceding siblings ...)
2018-09-18 9:51 ` [PATCH 00/35] exec: drop BQL from interrupt handling David Hildenbrand
@ 2018-09-20 20:05 ` Mark Cave-Ayland
6 siblings, 0 replies; 18+ messages in thread
From: Mark Cave-Ayland @ 2018-09-20 20:05 UTC (permalink / raw)
To: Emilio G. Cota, qemu-devel
Cc: Peter Maydell, Cornelia Huck, kvm, David Hildenbrand, James Hogan,
Anthony Green, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Alexander Graf, Christian Borntraeger, Richard Henderson,
Artyom Tarasenko, Eduardo Habkost, qemu-s390x, qemu-arm,
Stafford Horne, David Gibson, Chris Wulff, Peter Crosthwaite,
Marcelo Tosatti, Laurent Vivier, Michael Walle, qemu-ppc,
Aleksandar Markovic <amarkovic
On 17/09/2018 17:30, Emilio G. Cota wrote:
> This series comes originally from a series of patches that Paolo
> sent to me a long time ago. I have kept most of his S-o-b tags,
> but I have done the forward port of the patches to the current
> QEMU code base, so please blame all possible bugs on me, not him.
>
> The goal of this series is to push the BQL out of interrupt
> handling wherever possible. Many targets do not need the BQL
> to handle interrupts, so for those targets we get rid of the
> BQL in interrupt handling. In TCG mode, this by itself does
> not yield measurable performance gains. However, it is a
> necessary first step before tackling BQL contention when
> managing CPU states (i.e. exiting the exec loop always
> involves taking the BQL, which does not scale).
>
> The patches:
>
> - Patch 0 accesses icount_decr.u16 consistently with atomics.
> This is important because we use it across threads to signal
> that an interrupt is pending.
>
> - 2-28: convert direct callers of cpu->interrupt_request
> to use atomic accesses or CPU helper functions that use atomics.
> I have broken these up into individual patches to ease review.
> Note that in some cases I have added many atomic_reads, most
> of which are probably unnecessary. However, I'd leave this up
> to the maintainers, since I do not want to break things by
> doing a single atomic_read(&cpu->interrupt_request) at the
> beginning of a function and then miss subsequent updates to
> cpu->interrupt_request.
>
> - 29-31: drop the BQL requirement when handling cpu->interrupt_request
>
> - 32-33: drop now unnecessary BQL acquisitions in i386 and ppc
>
> - 34-35: push down the BQL acquisition to cpu->do_interrupt
> and cpu->cpu_exec_interrupt. Here I am Cc'ing everyone so that
> we can make sure I am not missing any target that requires
> the BQL on either do_interrupt or cpu_exec_interrupt.
>
> This series is checkpatch-clean, and can be fetched from:
> https://github.com/cota/qemu/tree/cpu-lock
>
> Note that the series applies on top of patches that are
> already on the qemu-devel list and will hopefully soon
> be upstream. This series begins after commit
> 4ece0cbd74 ("cpus: access .qemu_icount_bias with atomic64")
> in that tree.
I ran the above cpu-lock branch through my SPARC32 and SPARC64 test suite just to be
sure, and everything appeared to work fine. So for the SPARC32/64 patches:
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread