Re: [PATCH v2 0/7] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path
On Fri, 21 Aug 2020 at 06:56, Cornelia Huck wrote: > > > > As the first step in removing the BQL from this path, we will make > > changes to the core/common functions of cpu_handle_interrupt/exception > > to drop the holding of the BQL. The holding of the BQL is pushed down > > to the per-arch implementation code. > > I have only skimmed the patches I was cc:ed on so far, but the series > seems sane to me in general. Thanks for the feedback ! > > > > > This patch goes through several transitions of the code in order to > > maintain correctness (bisectability). In order to maintain > > bisectability across these steps some patches need to touch many > > files across different arches, however most of the changes are trivial. > > > > The general order of the changes is below where each step > > represents one patch. > > > > 1) rename all *_do_interrupt functions to *_do_interrupt_locked > > I'm wondering whether this renaming could be done in an automated way > (e.g. via Coccinelle). Reviewing the method for the renaming is often > easier than looking at a lot of similar code patterns. Good point. We will look into this. Thanks, -Rob Foley > > > > > 2) add a new function *_do_interrupt that takes the BQL and calls > > *_do_interrupt_locked, point ->do_interrupt to it, and remove > > the BQL from cpu-exec.c's cpu_handle_exception. > > > > 3) modify the BQL critical sections around > > ->cpu_exec_interrupt, so that the BQL critical section covers just the > > call to ->cpu_exec_interrupt. > > > > 4/5) same as 1/2 for ->cpu_exec_interrupt. This removes the BQL > > from cpu_handle_exception. > > The method of doing this in steps looks fine, although the patches > produced are a bit unwieldy -- that's something we have to live with, I > guess. >
Re: [PATCH-for-5.2] exec: Remove MemoryRegion::global_locking field
On Sat, 22 Aug 2020 at 16:13, Philippe Mathieu-Daudé wrote:
>
> +Robert
>
> On 8/7/20 12:16 PM, Paolo Bonzini wrote:
> > On 07/08/20 12:02, Stefan Hajnoczi wrote:
> >> On Thu, Aug 06, 2020 at 05:07:26PM +0200, Philippe Mathieu-Daudé wrote:
> >>> Last uses of memory_region_clear_global_locking() have been
> >>> removed in commit 7070e085d4 ("acpi: mark PMTIMER as unlocked")
> >>> and commit 08565552f7 ("cputlb: Move NOTDIRTY handling from I/O
> >>> path to TLB path").
> >>> Remove memory_region_clear_global_locking() and the now unused
> >>> 'global_locking' field in MemoryRegion.
> >>>
> >>> Reported-by: Alexander Bulekov
> >>> Suggested-by: Stefan Hajnoczi
> >>> Signed-off-by: Philippe Mathieu-Daudé
> >>> ---
> >>> include/exec/memory.h | 14 --
> >>> accel/tcg/cputlb.c| 4 ++--
> >>> exec.c| 2 +-
> >>> softmmu/memory.c | 6 --
> >>> 4 files changed, 3 insertions(+), 23 deletions(-)
> >>
> >> It can be added back in later, if necessary. For now let's drop the dead
> >> code.
> >>
> >> Reviewed-by: Stefan Hajnoczi
> >>
> >
> > I expect it will come back since Linaro is working on BQL-free interrupt
> > handling for TCG, but no objections.
>
> Robert, any comment on this patch?
>
> Thanks,
>
> Phil.
>
Phil, Thanks for the heads up on this !
No objections to removing this. We can easily re-add it if/when needed.
Thanks,
-Rob
[PATCH v2 7/7] target: Push BQL on ->cpu_exec_interrupt down into per-arch implementation
Push the BQL on ->cpu_exec_interrupt down into per-arch implementation.
The first step is to remove the BQL from cpu_handle_interrupt, which
currently calls ->cpu_exec_interrupt. We then added the *_cpu_exec_interrupt
functions, which get the BQL and then call to *_cpu_exec_interrupt_locked.
We also pointed the per-arch ->cpu_exec_interrupt at the new
*cpu_exec_interrupt.
This patch is part of a series of transitions to move the
BQL down into the cpu_exec_interrupt per arch functions. This set of
transitions is needed to maintain bisectability.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00784.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01517.html
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
accel/tcg/cpu-exec.c| 2 --
target/alpha/cpu.c | 2 +-
target/alpha/cpu.h | 2 +-
target/alpha/helper.c | 11 ++-
target/arm/cpu.c| 13 +++--
target/arm/cpu.h| 2 +-
target/arm/cpu64.c | 2 +-
target/arm/cpu_tcg.c| 11 ++-
target/avr/cpu.c| 2 +-
target/avr/cpu.h| 2 +-
target/avr/helper.c | 11 ++-
target/cris/cpu.c | 2 +-
target/cris/cpu.h | 2 +-
target/cris/helper.c| 11 ++-
target/hppa/cpu.c | 2 +-
target/hppa/cpu.h | 2 +-
target/hppa/int_helper.c| 11 ++-
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 +-
target/i386/seg_helper.c| 11 ++-
target/lm32/cpu.c | 2 +-
target/lm32/cpu.h | 2 +-
target/lm32/helper.c| 11 ++-
target/m68k/cpu.c | 2 +-
target/m68k/cpu.h | 2 +-
target/m68k/op_helper.c | 11 ++-
target/microblaze/cpu.c | 2 +-
target/microblaze/cpu.h | 2 +-
target/microblaze/helper.c | 11 ++-
target/mips/cpu.c | 2 +-
target/mips/helper.c| 11 ++-
target/mips/internal.h | 2 +-
target/nios2/cpu.c | 11 ++-
target/openrisc/cpu.c | 2 +-
target/openrisc/cpu.h | 2 +-
target/openrisc/interrupt.c | 12 +++-
target/ppc/cpu.h| 2 +-
target/ppc/excp_helper.c| 11 ++-
target/ppc/translate_init.inc.c | 2 +-
target/riscv/cpu.c | 2 +-
target/riscv/cpu.h | 2 +-
target/riscv/cpu_helper.c | 11 ++-
target/rx/cpu.c | 2 +-
target/rx/cpu.h | 2 +-
target/rx/helper.c | 11 ++-
target/s390x/cpu.c | 2 +-
target/s390x/excp_helper.c | 11 ++-
target/s390x/internal.h | 2 +-
target/sh4/cpu.c| 2 +-
target/sh4/cpu.h| 2 +-
target/sh4/helper.c | 12 +++-
target/sparc/cpu.c | 11 ++-
target/tilegx/cpu.c | 11 ++-
target/unicore32/cpu.c | 2 +-
target/unicore32/cpu.h | 2 +-
target/unicore32/helper.c | 11 ++-
target/xtensa/cpu.c | 2 +-
target/xtensa/cpu.h | 2 +-
target/xtensa/exc_helper.c | 12 +++-
59 files changed, 260 insertions(+), 61 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 499a8bdc5e..c4e54baa6f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -599,7 +599,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
True when it is, and we should restart on a new TB,
and via longjmp via cpu_loop_exit. */
else {
-qemu_mutex_lock_iothread();
if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
replay_interrupt();
/*
@@ -614,7 +613,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
/* The target hook may have updated the 'cpu->interrupt_request';
* reload the 'interrupt_request' value */
interrupt_request = cpu_interrupt_request(cpu);
-qemu_mutex_unlock_iothread();
}
if (interrupt_request & CPU_INTERRUPT_EXITTB) {
cpu_reset_interrupt(cpu, CPU_INTERRUPT_EXITTB);
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 623ed42e13..09677c6c44 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -218,7 +2
[PATCH v2 4/7] target: Push BQL on ->do_interrupt down into per-arch implementation
As part of pushing the BQL down into the per-arch implementation, the first change is to remove the holding of BQL from cpu_handle_exception. Next, we made changes per-arch to re-add a new *_do_interrupt function, which gets the BQL and then calls to *_do_interrupt_locked. We also pointed the per-arch ->do_interrupt at the new *_do_interrupt. This patch is part of a series of transitions to move the BQL down into the do_interrupt per arch functions. This set of transitions is needed to maintain bisectability. It is worth mentioning that arm and cris are slightly different. In a prior patch for these arches, we added a new CPUClass method ->do_interrupt_locked. The only difference for arm and cris is that their new *_do_interrupt functions will be able to utilize this new ->do_interrupt_locked method. avr is another exception. avr, arm and cris all had a similar case where their *_cpu_exec_interrupt was calling to the CPUClass ->do_interrupt. This causes an issue when we push the lock down since ->do_interrupt will try to acquire the BQL, but the calling context already has it. To solve this for arm and cris, we added a new CPUCLass method as explained above. Moreover, it was actually required for these arches since they have more than one possible value of ->do_interrupt. In the case of avr, there is only one possible value of ->do_interrupt, so for that reason we changed the avr_cpu_exec_interrupt to call directly to avr_cpu_do_interrupt_locked rather than call cc->do_interrupt. The purpose of this set of changes is to set the groundwork so that an arch could move towards removing the BQL from the cpu_handle_interrupt/exception paths. This approach was suggested by Paolo Bonzini. For reference, here are key posts in the discussion, explaining the reasoning/benefits of this approach. https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01517.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00784.html https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html Signed-off-by: Robert Foley --- accel/tcg/cpu-exec.c| 2 -- target/alpha/cpu.c | 2 +- target/alpha/cpu.h | 2 +- target/alpha/helper.c | 9 - target/arm/cpu.c| 2 +- target/arm/cpu.h| 2 ++ target/arm/cpu_tcg.c| 2 +- target/arm/helper.c | 8 target/avr/cpu.c| 2 +- target/avr/cpu.h| 2 +- target/avr/helper.c | 16 target/cris/cpu.c | 7 +-- target/cris/cpu.h | 1 + target/cris/helper.c| 8 target/hppa/cpu.c | 2 +- target/hppa/cpu.h | 2 +- target/hppa/int_helper.c| 9 - target/i386/cpu.c | 2 +- target/i386/cpu.h | 2 +- target/i386/seg_helper.c| 9 - target/lm32/cpu.c | 2 +- target/lm32/cpu.h | 2 +- target/lm32/helper.c| 9 - target/m68k/cpu.c | 2 +- target/m68k/cpu.h | 2 +- target/m68k/op_helper.c | 11 +-- target/microblaze/cpu.c | 2 +- target/microblaze/cpu.h | 2 +- target/microblaze/helper.c | 11 +-- target/mips/cpu.c | 2 +- target/mips/helper.c| 9 - target/mips/internal.h | 2 +- target/nios2/cpu.c | 2 +- target/nios2/cpu.h | 1 + target/nios2/helper.c | 9 + target/openrisc/cpu.c | 2 +- target/openrisc/cpu.h | 2 +- target/openrisc/interrupt.c | 9 - target/ppc/cpu.h| 1 + target/ppc/excp_helper.c| 7 +++ target/ppc/translate_init.inc.c | 2 +- target/riscv/cpu.c | 2 +- target/riscv/cpu.h | 2 +- target/riscv/cpu_helper.c | 11 ++- target/rx/cpu.c | 2 +- target/rx/cpu.h | 1 + target/rx/helper.c | 7 +++ target/s390x/cpu.c | 2 +- target/s390x/excp_helper.c | 7 +++ target/s390x/internal.h | 1 + target/sh4/cpu.c| 2 +- target/sh4/cpu.h| 2 +- target/sh4/helper.c | 11 +-- target/sparc/cpu.c | 2 +- target/sparc/cpu.h | 1 + target/sparc/int32_helper.c | 7 +++ target/sparc/int64_helper.c | 7 +++ target/tilegx/cpu.c | 9 - target/unicore32/cpu.c | 2 +- target/unicore32/cpu.h | 1 + target/unicore32/softmmu.c | 7 +++ target/xtensa/cpu.c | 2 +- target/xtensa/cpu.h | 2 +- target/xtensa/exc_helper.c | 11 +-- 64 files changed, 223 insertions(+), 60 deletions(-) di
[PATCH v2 5/7] accel/tcg: Change BQL critical section in cpu_handle_interrupt
We are changing the critical section from being around
the majority of the cpu_handle_interrupt to instead
be around just the call to ->cpu_exec_interrupt.
This is in preparation for pushing down the BQL into the
per arch implementation.
We should mention that we discussed these changes as well as
some open questions here:
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01189.html
With the main open question being:
BQL is clearly needed to protect the critical section around the call to
->cpu_exec_interrupt. What else is the BQL protecting in cpu_handle_interrupt
that we need to consider? Are we missing anything here?
Signed-off-by: Robert Foley
---
accel/tcg/cpu-exec.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index e661635f06..499a8bdc5e 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -556,7 +556,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
if (unlikely(cpu_interrupt_request(cpu))) {
int interrupt_request;
-qemu_mutex_lock_iothread();
interrupt_request = cpu_interrupt_request(cpu);
if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
/* Mask out external interrupts for this step. */
@@ -565,7 +564,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()) {
@@ -575,13 +573,13 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
cpu_halted_set(cpu, 1);
cpu->exception_index = EXCP_HLT;
-qemu_mutex_unlock_iothread();
return true;
}
#if defined(TARGET_I386)
else if (interrupt_request & CPU_INTERRUPT_INIT) {
X86CPU *x86_cpu = X86_CPU(cpu);
CPUArchState *env = _cpu->env;
+qemu_mutex_lock_iothread();
replay_interrupt();
cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
do_cpu_init(x86_cpu);
@@ -593,7 +591,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
else if (interrupt_request & CPU_INTERRUPT_RESET) {
replay_interrupt();
cpu_reset(cpu);
-qemu_mutex_unlock_iothread();
return true;
}
#endif
@@ -602,6 +599,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
True when it is, and we should restart on a new TB,
and via longjmp via cpu_loop_exit. */
else {
+qemu_mutex_lock_iothread();
if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
replay_interrupt();
/*
@@ -616,6 +614,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
/* The target hook may have updated the 'cpu->interrupt_request';
* reload the 'interrupt_request' value */
interrupt_request = cpu_interrupt_request(cpu);
+qemu_mutex_unlock_iothread();
}
if (interrupt_request & CPU_INTERRUPT_EXITTB) {
cpu_reset_interrupt(cpu, CPU_INTERRUPT_EXITTB);
@@ -625,7 +624,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
}
/* 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. */
--
2.17.1
[PATCH v2 3/7] target/cris: add CRISCPUClass->do_interrupt_locked
Adding ->do_interrupt_locked to CRISCPUClass is preparation for
pushing the BQL down into the per-arch implementation of ->do_interrupt.
This is needed since Cris's *_cpu_exec_interrupt calls to *_do_interrupt.
With the push down of the BQL into *_cpu_exec_interrupt and
*_do_interrupt, *_cpu_exec_interrupt will call to ->do_interrupt
with lock held. Since ->do_interrupt also has the lock, we need a way
to allow cpu_exec_interrupt to call do_interrupt with lock held.
This patch solves the issue of *_cpu_exec_interrupt needing
to call do_interrupt with lock held.
This patch is part of a series of transitions to move the
BQL down into the do_interrupt per arch functions. This set of
transitions is needed to maintain bisectability.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00784.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01517.html
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/cris/cpu-qom.h | 3 +++
target/cris/cpu.c | 6 ++
target/cris/helper.c | 6 +++---
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/target/cris/cpu-qom.h b/target/cris/cpu-qom.h
index f1de6041dc..dc94a17ffe 100644
--- a/target/cris/cpu-qom.h
+++ b/target/cris/cpu-qom.h
@@ -36,6 +36,7 @@
* @parent_realize: The parent class' realize handler.
* @parent_reset: The parent class' reset handler.
* @vr: Version Register value.
+ * @do_interrupt_locked: Handler for interrupts (lock already held).
*
* A CRIS CPU model.
*/
@@ -48,6 +49,8 @@ typedef struct CRISCPUClass {
DeviceReset parent_reset;
uint32_t vr;
+
+void (*do_interrupt_locked)(CPUState *cpu);
} CRISCPUClass;
typedef struct CRISCPU CRISCPU;
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index 40b110f161..948eeb6260 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -200,6 +200,7 @@ static void crisv8_cpu_class_init(ObjectClass *oc, void
*data)
ccc->vr = 8;
cc->do_interrupt = crisv10_cpu_do_interrupt_locked;
+ccc->do_interrupt_locked = crisv10_cpu_do_interrupt_locked;
cc->gdb_read_register = crisv10_cpu_gdb_read_register;
cc->tcg_initialize = cris_initialize_crisv10_tcg;
}
@@ -211,6 +212,7 @@ static void crisv9_cpu_class_init(ObjectClass *oc, void
*data)
ccc->vr = 9;
cc->do_interrupt = crisv10_cpu_do_interrupt_locked;
+ccc->do_interrupt_locked = crisv10_cpu_do_interrupt_locked;
cc->gdb_read_register = crisv10_cpu_gdb_read_register;
cc->tcg_initialize = cris_initialize_crisv10_tcg;
}
@@ -222,6 +224,7 @@ static void crisv10_cpu_class_init(ObjectClass *oc, void
*data)
ccc->vr = 10;
cc->do_interrupt = crisv10_cpu_do_interrupt_locked;
+ccc->do_interrupt_locked = crisv10_cpu_do_interrupt_locked;
cc->gdb_read_register = crisv10_cpu_gdb_read_register;
cc->tcg_initialize = cris_initialize_crisv10_tcg;
}
@@ -233,6 +236,7 @@ static void crisv11_cpu_class_init(ObjectClass *oc, void
*data)
ccc->vr = 11;
cc->do_interrupt = crisv10_cpu_do_interrupt_locked;
+ccc->do_interrupt_locked = crisv10_cpu_do_interrupt_locked;
cc->gdb_read_register = crisv10_cpu_gdb_read_register;
cc->tcg_initialize = cris_initialize_crisv10_tcg;
}
@@ -244,6 +248,7 @@ static void crisv17_cpu_class_init(ObjectClass *oc, void
*data)
ccc->vr = 17;
cc->do_interrupt = crisv10_cpu_do_interrupt_locked;
+ccc->do_interrupt_locked = crisv10_cpu_do_interrupt_locked;
cc->gdb_read_register = crisv10_cpu_gdb_read_register;
cc->tcg_initialize = cris_initialize_crisv10_tcg;
}
@@ -269,6 +274,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
cc->class_by_name = cris_cpu_class_by_name;
cc->has_work = cris_cpu_has_work;
cc->do_interrupt = cris_cpu_do_interrupt_locked;
+ccc->do_interrupt_locked = cris_cpu_do_interrupt_locked;
cc->cpu_exec_interrupt = cris_cpu_exec_interrupt;
cc->dump_state = cris_cpu_dump_state;
cc->set_pc = cris_cpu_set_pc;
diff --git a/target/cris/helper.c b/target/cris/helper.c
index e0ee6b4e05..3b7ee74813 100644
--- a/target/cris/helper.c
+++ b/target/cris/helper.c
@@ -290,7 +290,7 @@ hwaddr cris_cpu_get_phys_page_debug(CPUState *cs, vaddr
addr)
bool cris_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
-CPUClass *cc = CPU_GET_CLASS(cs);
+CRISCPUClass *ccc = CRIS_CPU_CLASS(cs);
CRISCPU *cpu = CRIS_CPU(cs);
CPUCRISState *env = >env;
bool ret = false;
@@ -299,7 +299,7 @@ bool cris_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
&& (env->pregs[PR_CCS] & I_FLAG)
&
[PATCH v2 1/7] target: rename all *_do_interupt functions to _do_interrupt_locked
The rename of all *_do_interrupt functions to *_do_interrupt_locked
is preparation for pushing the BQL lock around these functions
down into the per-arch implementation of *_do_interrupt.
In a later patch which pushes down the lock, we will add
a new *_do_interrupt function which grabs the BQL and calls to
*_do_interrupt_locked.
This is the first patch in a series of transitions to move the
BQL down into the do_interrupt per arch function. This set of
transitions is needed to maintain bisectability.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00784.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01517.html
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
hw/ppc/spapr_events.c | 2 +-
target/alpha/cpu.c | 2 +-
target/alpha/cpu.h | 2 +-
target/alpha/helper.c | 4 ++--
target/arm/cpu.c| 2 +-
target/arm/cpu.h| 4 ++--
target/arm/cpu_tcg.c| 2 +-
target/arm/helper.c | 2 +-
target/arm/m_helper.c | 2 +-
target/avr/cpu.c| 2 +-
target/avr/cpu.h| 2 +-
target/avr/helper.c | 2 +-
target/cris/cpu.c | 12 ++--
target/cris/cpu.h | 4 ++--
target/cris/helper.c| 10 +-
target/hppa/cpu.c | 2 +-
target/hppa/cpu.h | 2 +-
target/hppa/int_helper.c| 4 ++--
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 +-
target/i386/seg_helper.c| 2 +-
target/lm32/cpu.c | 2 +-
target/lm32/cpu.h | 2 +-
target/lm32/helper.c| 4 ++--
target/m68k/cpu.c | 2 +-
target/m68k/cpu.h | 2 +-
target/m68k/op_helper.c | 4 ++--
target/microblaze/cpu.c | 2 +-
target/microblaze/cpu.h | 2 +-
target/microblaze/helper.c | 6 +++---
target/mips/cpu.c | 2 +-
target/mips/helper.c| 4 ++--
target/mips/internal.h | 2 +-
target/moxie/cpu.c | 2 +-
target/moxie/cpu.h | 2 +-
target/moxie/helper.c | 2 +-
target/nios2/cpu.c | 4 ++--
target/nios2/cpu.h | 2 +-
target/nios2/helper.c | 4 ++--
target/openrisc/cpu.c | 2 +-
target/openrisc/cpu.h | 2 +-
target/openrisc/interrupt.c | 4 ++--
target/ppc/cpu.h| 2 +-
target/ppc/excp_helper.c| 4 ++--
target/ppc/kvm.c| 2 +-
target/ppc/translate_init.inc.c | 2 +-
target/riscv/cpu.c | 2 +-
target/riscv/cpu.h | 2 +-
target/riscv/cpu_helper.c | 4 ++--
target/rx/cpu.c | 2 +-
target/rx/cpu.h | 2 +-
target/rx/helper.c | 4 ++--
target/s390x/cpu.c | 2 +-
target/s390x/excp_helper.c | 6 +++---
target/s390x/internal.h | 2 +-
target/sh4/cpu.c| 2 +-
target/sh4/cpu.h| 2 +-
target/sh4/helper.c | 6 +++---
target/sparc/cpu.c | 4 ++--
target/sparc/cpu.h | 2 +-
target/sparc/int32_helper.c | 2 +-
target/sparc/int64_helper.c | 2 +-
target/tilegx/cpu.c | 6 +++---
target/unicore32/cpu.c | 2 +-
target/unicore32/cpu.h | 2 +-
target/unicore32/helper.c | 2 +-
target/unicore32/softmmu.c | 2 +-
target/xtensa/cpu.c | 2 +-
target/xtensa/cpu.h | 2 +-
target/xtensa/exc_helper.c | 6 +++---
70 files changed, 103 insertions(+), 103 deletions(-)
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 1069d0197b..b281022e20 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -879,7 +879,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
if (spapr->fwnmi_machine_check_addr == -1) {
/* Non-FWNMI case, deliver it like an architected CPU interrupt. */
cs->exception_index = POWERPC_EXCP_MCHECK;
-ppc_cpu_do_interrupt(cs);
+ppc_cpu_do_interrupt_locked(cs);
return;
}
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 09677c6c44..cb1074e0f9 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -217,7 +217,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void
*data)
cc->class_by_name = alpha_cpu_class_by_name;
cc->has_work = alpha_cpu_has_work;
-cc->do_interrupt = alpha_cpu_do_int
[PATCH v2 2/7] target/arm: add ARMCPUClass->do_interrupt_locked
Adding ->do_interrupt_locked to ARMCPUClass is preparation for
pushing the BQL down into the per-arch implementation of ->do_interrupt.
This is needed since ARM's *_cpu_exec_interrupt calls to *_do_interrupt.
With the push down of the BQL into *_cpu_exec_interrupt and
*_do_interrupt, *_cpu_exec_interrupt will call to ->do_interrupt
with lock held. Since ->do_interrupt also has the lock, we need a way
to allow cpu_exec_interrupt to call do_interrupt with lock held.
This patch solves this issue of *_cpu_exec_interrupt needing
to call do_interrupt with lock held.
This patch is part of a series of transitions to move the
BQL down into the do_interrupt per arch functions. This set of
transitions is needed to maintain bisectability.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00784.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01517.html
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/arm/cpu-qom.h | 3 +++
target/arm/cpu.c | 5 +++--
target/arm/cpu_tcg.c | 5 +++--
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 56395b87f6..264280194c 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -48,6 +48,7 @@ void aarch64_cpu_register(const ARMCPUInfo *info);
* ARMCPUClass:
* @parent_realize: The parent class' realize handler.
* @parent_reset: The parent class' reset handler.
+ * @do_interrupt_locked: Handler for interrupts (lock already held).
*
* An ARM CPU model.
*/
@@ -59,6 +60,8 @@ typedef struct ARMCPUClass {
const ARMCPUInfo *info;
DeviceRealize parent_realize;
DeviceReset parent_reset;
+
+void (*do_interrupt_locked)(CPUState *cpu);
} ARMCPUClass;
typedef struct ARMCPU ARMCPU;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 46c1d92080..d15b459399 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -526,7 +526,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned
int excp_idx,
bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
-CPUClass *cc = CPU_GET_CLASS(cs);
+ARMCPUClass *acc = ARM_CPU_GET_CLASS(cs);
CPUARMState *env = cs->env_ptr;
uint32_t cur_el = arm_current_el(env);
bool secure = arm_is_secure(env);
@@ -573,7 +573,7 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
found:
cs->exception_index = excp_idx;
env->exception.target_el = target_el;
-cc->do_interrupt(cs);
+acc->do_interrupt_locked(cs);
return true;
}
@@ -2225,6 +2225,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void
*data)
cc->gdb_write_register = arm_cpu_gdb_write_register;
#ifndef CONFIG_USER_ONLY
cc->do_interrupt = arm_cpu_do_interrupt_locked;
+acc->do_interrupt_locked = arm_cpu_do_interrupt_locked;
cc->get_phys_page_attrs_debug = arm_cpu_get_phys_page_attrs_debug;
cc->asidx_from_attrs = arm_asidx_from_attrs;
cc->vmsd = _arm_cpu;
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 2fc7a29340..caf0d54c2c 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -17,7 +17,7 @@
static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
-CPUClass *cc = CPU_GET_CLASS(cs);
+ARMCPUClass *acc = ARM_CPU_GET_CLASS(cs);
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = >env;
bool ret = false;
@@ -33,7 +33,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
if (interrupt_request & CPU_INTERRUPT_HARD
&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
cs->exception_index = EXCP_IRQ;
-cc->do_interrupt(cs);
+acc->do_interrupt_locked(cs);
ret = true;
}
return ret;
@@ -602,6 +602,7 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data)
acc->info = data;
#ifndef CONFIG_USER_ONLY
cc->do_interrupt = arm_v7m_cpu_do_interrupt_locked;
+acc->do_interrupt_locked = arm_v7m_cpu_do_interrupt_locked;
#endif
cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt;
--
2.17.1
[PATCH v2 6/7] target: rename all *_cpu_exec_interrupt functions to *_cpu_exec_interrupt_locked
The rename of all *_cpu_exec_interrupt functions to *_cpu_exec_interrupt_locked is preparation for pushing the BQL lock around these functions down into the per-arch implementation of *_cpu_exec_interrupt. In a later patch, which pushes down the lock, we will add a new *_cpu_exec_interrupt function, which grabs the BQL and calls to *_cpu_exec_interrupt_locked. This patch is part of a series of transitions to move the BQL down into the cpu_exec_interrupt per arch functions. This set of transitions is needed to maintain bisectability. The purpose of this set of changes is to set the groundwork so that an arch could move towards removing the BQL from the cpu_handle_interrupt/exception paths. This approach was suggested by Paolo Bonzini. For reference, here are key posts in the discussion, explaining the reasoning/benefits of this approach. https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00784.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01517.html https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html Signed-off-by: Robert Foley --- target/alpha/cpu.c | 2 +- target/alpha/cpu.h | 2 +- target/alpha/helper.c | 2 +- target/arm/cpu.c| 4 ++-- target/arm/cpu.h| 2 +- target/arm/cpu64.c | 2 +- target/arm/cpu_tcg.c| 5 +++-- target/avr/cpu.c| 2 +- target/avr/cpu.h| 2 +- target/avr/helper.c | 2 +- target/cris/cpu.c | 2 +- target/cris/cpu.h | 2 +- target/cris/helper.c| 2 +- target/hppa/cpu.c | 2 +- target/hppa/cpu.h | 2 +- target/hppa/int_helper.c| 2 +- target/i386/cpu.c | 2 +- target/i386/cpu.h | 2 +- target/i386/seg_helper.c| 2 +- target/lm32/cpu.c | 2 +- target/lm32/cpu.h | 2 +- target/lm32/helper.c| 2 +- target/m68k/cpu.c | 2 +- target/m68k/cpu.h | 2 +- target/m68k/op_helper.c | 2 +- target/microblaze/cpu.c | 2 +- target/microblaze/cpu.h | 2 +- target/microblaze/helper.c | 2 +- target/mips/cpu.c | 2 +- target/mips/helper.c| 2 +- target/mips/internal.h | 2 +- target/nios2/cpu.c | 4 ++-- target/openrisc/cpu.c | 2 +- target/openrisc/cpu.h | 2 +- target/openrisc/interrupt.c | 2 +- target/ppc/cpu.h| 2 +- target/ppc/excp_helper.c| 2 +- target/ppc/translate_init.inc.c | 2 +- target/riscv/cpu.c | 2 +- target/riscv/cpu.h | 2 +- target/riscv/cpu_helper.c | 2 +- target/rx/cpu.c | 2 +- target/rx/cpu.h | 2 +- target/rx/helper.c | 2 +- target/s390x/cpu.c | 2 +- target/s390x/excp_helper.c | 2 +- target/s390x/internal.h | 2 +- target/sh4/cpu.c| 2 +- target/sh4/cpu.h| 2 +- target/sh4/helper.c | 2 +- target/sparc/cpu.c | 4 ++-- target/tilegx/cpu.c | 5 +++-- target/unicore32/cpu.c | 2 +- target/unicore32/cpu.h | 2 +- target/unicore32/helper.c | 2 +- target/xtensa/cpu.c | 2 +- target/xtensa/cpu.h | 2 +- target/xtensa/exc_helper.c | 2 +- 58 files changed, 65 insertions(+), 63 deletions(-) diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c index 09677c6c44..623ed42e13 100644 --- a/target/alpha/cpu.c +++ b/target/alpha/cpu.c @@ -218,7 +218,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void *data) cc->class_by_name = alpha_cpu_class_by_name; cc->has_work = alpha_cpu_has_work; cc->do_interrupt = alpha_cpu_do_interrupt; -cc->cpu_exec_interrupt = alpha_cpu_exec_interrupt; +cc->cpu_exec_interrupt = alpha_cpu_exec_interrupt_locked; cc->dump_state = alpha_cpu_dump_state; cc->set_pc = alpha_cpu_set_pc; cc->gdb_read_register = alpha_cpu_gdb_read_register; diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h index be29bdd530..9ed82b5785 100644 --- a/target/alpha/cpu.h +++ b/target/alpha/cpu.h @@ -277,7 +277,7 @@ extern const VMStateDescription vmstate_alpha_cpu; #endif void alpha_cpu_do_interrupt(CPUState *cpu); -bool alpha_cpu_exec_interrupt(CPUState *cpu, int int_req); +bool alpha_cpu_exec_interrupt_locked(CPUState *cpu, int int_req); void alpha_cpu_dump_state(CPUState *cs, FILE *f, int flags); hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr); int alpha_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); diff --git a/target/alpha/helper.c b/target/alpha/helper.c index e497dd269e..b5fa849f0f 100644 --- a/target/alpha/helper.c +++ b/target/alpha/helper.c @@ -414,7 +414,7 @@ void alpha_cpu_d
[PATCH v2 0/7] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path
The purpose of this change is to set the groundwork so that an arch could move towards removing the BQL from the cpu_handle_interrupt/exception paths. The BQL is a bottleneck in scaling to more cores. And this cpu_handle_interrupt/exception path is one of the key BQL users as measured by the QEMU sync profiling (qsp). As the first step in removing the BQL from this path, we will make changes to the core/common functions of cpu_handle_interrupt/exception to drop the holding of the BQL. The holding of the BQL is pushed down to the per-arch implementation code. This patch goes through several transitions of the code in order to maintain correctness (bisectability). In order to maintain bisectability across these steps some patches need to touch many files across different arches, however most of the changes are trivial. The general order of the changes is below where each step represents one patch. 1) rename all *_do_interrupt functions to *_do_interrupt_locked 2) add a new function *_do_interrupt that takes the BQL and calls *_do_interrupt_locked, point ->do_interrupt to it, and remove the BQL from cpu-exec.c's cpu_handle_exception. 3) modify the BQL critical sections around ->cpu_exec_interrupt, so that the BQL critical section covers just the call to ->cpu_exec_interrupt. 4/5) same as 1/2 for ->cpu_exec_interrupt. This removes the BQL from cpu_handle_exception. This approach of pushing the BQL down to the per arch functions was suggested by Paolo Bonzini. For reference, here are several key posts in the discussion, explaining the reasoning/benefits of this approach. https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00784.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01517.html https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html This patch series is based on the per-CPU locks patch: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05314.html Our most recent WIP tree is here: https://github.com/rf972/qemu/tree/interrupts_v2.7 Robert Foley (7): target: rename all *_do_interupt functions to _do_interrupt_locked target/arm: add ARMCPUClass->do_interrupt_locked target/cris: add CRISCPUClass->do_interrupt_locked target: Push BQL on ->do_interrupt down into per-arch implementation accel/tcg: Change BQL critical section in cpu_handle_interrupt target: rename all *_cpu_exec_interrupt functions to *_cpu_exec_interrupt_locked target: Push BQL on ->cpu_exec_interrupt down into per-arch implementation accel/tcg/cpu-exec.c| 8 +--- hw/ppc/spapr_events.c | 2 +- target/alpha/helper.c | 22 +++--- target/arm/cpu-qom.h| 3 +++ target/arm/cpu.c| 16 +--- target/arm/cpu.h| 2 ++ target/arm/cpu_tcg.c| 19 +++ target/arm/helper.c | 10 +- target/arm/m_helper.c | 2 +- target/avr/helper.c | 27 ++- target/cris/cpu-qom.h | 3 +++ target/cris/cpu.c | 11 ++- target/cris/cpu.h | 3 ++- target/cris/helper.c| 35 ++- target/hppa/int_helper.c| 22 +++--- target/i386/seg_helper.c| 20 ++-- target/lm32/helper.c| 22 +++--- target/m68k/op_helper.c | 22 +++--- target/microblaze/helper.c | 24 target/mips/helper.c| 22 +++--- target/moxie/cpu.c | 2 +- target/moxie/cpu.h | 2 +- target/moxie/helper.c | 2 +- target/nios2/cpu.c | 13 +++-- target/nios2/cpu.h | 1 + target/nios2/helper.c | 13 +++-- target/openrisc/interrupt.c | 23 --- target/ppc/cpu.h| 1 + target/ppc/excp_helper.c| 22 +++--- target/ppc/kvm.c| 2 +- target/riscv/cpu_helper.c | 24 +--- target/rx/cpu.h | 1 + target/rx/helper.c | 22 +++--- target/s390x/excp_helper.c | 24 target/s390x/internal.h | 1 + target/sh4/helper.c | 25 + target/sparc/cpu.c | 13 +++-- target/sparc/cpu.h | 1 + target/sparc/int32_helper.c | 9 - target/sparc/int64_helper.c | 9 - target/tilegx/cpu.c | 23 --- target/unicore32/cpu.h | 1 + target/unicore32/helper.c | 13 +++-- target/unicore32/softmmu.c | 9 - target/xtensa/exc_helper.c | 25 + 45 files changed, 476 insertions(+), 100 deletions(-) -- 2.17.1
Re: [PATCH 007/147] tests/vm: do not pollute configure with --efi-aarch64
Reviewed-by: Robert Foley
On Mon, 10 Aug 2020 at 13:12, Paolo Bonzini wrote:
>
> Just make EFI_AARCH64 a variable in the makefile that defaults to the efi
> firmware included with QEMU. It can be redefined on the "make" command
> line.
>
> Signed-off-by: Paolo Bonzini
> ---
> configure | 19 ---
> tests/vm/Makefile.include | 2 ++
> 2 files changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/configure b/configure
> index 2acc4d1..1c17a0f 100755
> --- a/configure
> +++ b/configure
> @@ -418,7 +418,6 @@ prefix="/usr/local"
> mandir="\${prefix}/share/man"
> datadir="\${prefix}/share"
> firmwarepath="\${prefix}/share/qemu-firmware"
> -efi_aarch64=""
> qemu_docdir="\${prefix}/share/doc/qemu"
> bindir="\${prefix}/bin"
> libdir="\${prefix}/lib"
> @@ -1109,8 +1108,6 @@ for opt do
>;;
>--firmwarepath=*) firmwarepath="$optarg"
>;;
> - --efi-aarch64=*) efi_aarch64="$optarg"
> - ;;
>--host=*|--build=*|\
>--disable-dependency-tracking|\
>--sbindir=*|--sharedstatedir=*|\
> @@ -3650,20 +3647,6 @@ EOF
>fi
> fi
>
> -
> -# efi-aarch64 probe
> -# Check for efi files needed by aarch64 VMs.
> -# By default we will use the efi included with QEMU.
> -# Allow user to override the path for efi also.
> -if ! test -f "$efi_aarch64"; then
> - if test -f $source_path/pc-bios/edk2-aarch64-code.fd.bz2; then
> -# valid after build
> -efi_aarch64=$PWD/pc-bios/edk2-aarch64-code.fd
> - else
> -efi_aarch64=""
> - fi
> -fi
> -
> ##
> # libcap-ng library probe
> if test "$cap_ng" != "no" ; then
> @@ -6861,7 +6844,6 @@ if test "$docs" != "no"; then
> echo "sphinx-build $sphinx_build"
> fi
> echo "genisoimage $genisoimage"
> -echo "efi_aarch64 $efi_aarch64"
> echo "python_yaml $python_yaml"
> echo "slirp support $slirp $(echo_version $slirp $slirp_version)"
> if test "$slirp" != "no" ; then
> @@ -7963,7 +7945,6 @@ echo "PYTHON=$python" >> $config_host_mak
> echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak
> echo "SPHINX_WERROR=$sphinx_werror" >> $config_host_mak
> echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
> -echo "EFI_AARCH64=$efi_aarch64" >> $config_host_mak
> echo "PYTHON_YAML=$python_yaml" >> $config_host_mak
> echo "CC=$cc" >> $config_host_mak
> if $iasl -h > /dev/null 2>&1; then
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index f21948c..a599d19 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -2,6 +2,8 @@
>
> .PHONY: vm-build-all vm-clean-all
>
> +EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)
> +
> IMAGES := freebsd netbsd openbsd centos fedora
> ifneq ($(GENISOIMAGE),)
> IMAGES += ubuntu.i386 centos
> --
> 1.8.3.1
>
>
>
Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
On Sat, 8 Aug 2020 at 08:00, Paolo Bonzini wrote:
> > We are thinking that 2) would be a good option.
>
> Yes, it is. The only slight complication is that you'd have both
> ->do_interrupt and ->do_interrupt_locked so you probably should add some
> consistency check, for example
>
> /*
> * cc->do_interrupt_locked should only be needed if
> * the class uses cpu_common_do_interrupt.
> */
> assert(cc->do_interrupt == cpu_common_do_interrupt ||
>!cc->do_interrupt_locked);
>
> Therefore, a variant is to add ->do_interrupt_locked to ARMCPUClass and
> CRISCPUClass (target/avr/helper.c can just call
> avr_cpu_do_interrupt_locked, because that's the only value that
> cc->do_interrupt can have). Then ARM and CRIS can have a do_interrupt
> like you wrote above:
>
> void arm_do_interrupt(CPUState *cs)
> {
> ARMCPUClass *acc = ARM_CPU_GET_CLASS(cs);
> qemu_mutex_lock_iothread();
> acc->do_interrupt_locked(cpu);
> qemu_mutex_unlock_iothread();
> }
>
> with a small duplication between ARM and CRIS but on the other hand a
> simpler definition of the common CPUClass.
Thanks for all the details! It sounds like a good approach and we will
move forward with it.
Thanks & Regards,
-Rob
>
> Paolo
>
Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
We found cases where a few of the *_cpu_exec_interrupt per arch functions
call CPU class's cc->do_interrupt function pointer (for example
arm_cpu_exec_interrupt does this).
This is an issue because *_cpu_exec_interrupt will hold the BQL across the
call to cc->do_interrupt, and *_do_interrupt will also hold the BQL.
Most of the arches do not have this issue because they call the *_do_interrupt
function for that arch directly, and in those cases we will change to call
the function *_do_interrupt_locked.
We see a few possible solutions to this:
1) We could go back to the option of conditionalizing the BQL inside
these few *_do_interrupt functions, only acquiring the BQL if it is not
already held. I counted 3 different arches that directly use the
->do_interrupt
function from their *_cpu_exec_interrupt.
2) Another perhaps cleaner option is to add a new cpu class function
->do_interrupt_locked.
This lets callers like *_cpu_exec_interrupt call to ->do_interrupt_locked
with lock held and solves the issue without resorting to conditional locking.
Another benefit we could gain from this approach is to simplify our solution
overall by adding a common do_interrupt function.
void cpu_common_do_interrupt(CPUState *cs)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
qemu_mutex_lock_iothread();
cc->do_interrupt_locked(cpu);
qemu_mutex_unlock_iothread();
}
cc->do_interrupt would be set to cpu_common_do_interrupt by default
in cpu_class_init.
In other words, the base cpu class would handle holding the BQL for us,
and we would not need to implement a new *_do_interrupt function
for each arch.
We are thinking that 2) would be a good option.
What are the opinions on these possible solutions? Or are there other
solutions that we should consider here?
Thanks & Regards,
-Rob
On Thu, 6 Aug 2020 at 16:04, Robert Foley wrote:
>
> The comment around documenting the cpu_mutex fields and critical sections
> got us thinking and revisiting our locking assumptions in
> cpu_handle_interrupt.
>
> Initially we were thinking that removing the BQL from cpu_handle_interrupt
> meant that we needed to replace it with the cpu mutex to protect the per cpu
> data that is accessed like interrupt_request. We are reconsidering this and
> now thinking that the cpu mutex might not be needed here.
>
> BQL is clearly needed to protect the critical section around the call to
> ->cpu_exec_interrupt. What else is the BQL protecting in cpu_handle_interrupt
> that we need to consider? Are we missing anything here?
>
> It's also worth mentioning that we did give this approach a try.
> We tried out changes to cpu_handle_interrupt(),
> dropping the BQL from all but around ->cpu_exec_interrupt, and not using the
> cpu_mutex at all. It seemed to be functional, passing all the tests that
> we tried (so far). :)
>
> Thanks,
> -Rob
>
> On Thu, 6 Aug 2020 at 12:11, Robert Foley wrote:
> >
> > On Thu, 6 Aug 2020 at 05:22, Paolo Bonzini wrote:
> > >
> > > On 05/08/20 21:18, Richard Henderson wrote:
> > > > On 8/5/20 11:12 AM, Robert Foley wrote:
> > > >> This change removes the implied BQL from the cpu_handle_interrupt,
> > > >> and cpu_handle_exception paths. This BQL acquire is being pushed
> > > >> down into the per arch implementation.
> > > >>
> > > >> Signed-off-by: Robert Foley
> > > >> ---
> > > >> accel/tcg/cpu-exec.c | 19 +++
> > > >> 1 file changed, 11 insertions(+), 8 deletions(-)
> > > >>
> > > >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > > >> index 80d0e649b2..8e2bfd97a1 100644
> > > >> --- a/accel/tcg/cpu-exec.c
> > > >> +++ b/accel/tcg/cpu-exec.c
> > > >> @@ -517,9 +517,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;
> > > >>
> > > >
> > > > This patch is not bisectable. The removal of the lock here needs to
> > > > happen at
> > > > the end, or something.
> > >
> > > Indeed the series should be structured like this:
> > >
> > > 1) rename all *_do_interrupt functions to *_do_interrupt_locked
> > >
> >
Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
The comment around documenting the cpu_mutex fields and critical sections
got us thinking and revisiting our locking assumptions in cpu_handle_interrupt.
Initially we were thinking that removing the BQL from cpu_handle_interrupt
meant that we needed to replace it with the cpu mutex to protect the per cpu
data that is accessed like interrupt_request. We are reconsidering this and
now thinking that the cpu mutex might not be needed here.
BQL is clearly needed to protect the critical section around the call to
->cpu_exec_interrupt. What else is the BQL protecting in cpu_handle_interrupt
that we need to consider? Are we missing anything here?
It's also worth mentioning that we did give this approach a try.
We tried out changes to cpu_handle_interrupt(),
dropping the BQL from all but around ->cpu_exec_interrupt, and not using the
cpu_mutex at all. It seemed to be functional, passing all the tests that
we tried (so far). :)
Thanks,
-Rob
On Thu, 6 Aug 2020 at 12:11, Robert Foley wrote:
>
> On Thu, 6 Aug 2020 at 05:22, Paolo Bonzini wrote:
> >
> > On 05/08/20 21:18, Richard Henderson wrote:
> > > On 8/5/20 11:12 AM, Robert Foley wrote:
> > >> This change removes the implied BQL from the cpu_handle_interrupt,
> > >> and cpu_handle_exception paths. This BQL acquire is being pushed
> > >> down into the per arch implementation.
> > >>
> > >> Signed-off-by: Robert Foley
> > >> ---
> > >> accel/tcg/cpu-exec.c | 19 +++
> > >> 1 file changed, 11 insertions(+), 8 deletions(-)
> > >>
> > >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > >> index 80d0e649b2..8e2bfd97a1 100644
> > >> --- a/accel/tcg/cpu-exec.c
> > >> +++ b/accel/tcg/cpu-exec.c
> > >> @@ -517,9 +517,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;
> > >>
> > >
> > > This patch is not bisectable. The removal of the lock here needs to
> > > happen at
> > > the end, or something.
> >
> > Indeed the series should be structured like this:
> >
> > 1) rename all *_do_interrupt functions to *_do_interrupt_locked
> >
> > 2) add back *_do_interrupt that takes the BQL and calls
> > *_do_interrupt_locked, point ->do_interrupt to it, remove the BQL from
> > cpu-exec.c
> >
> > 3) modify the cpu_mutex and BQL critical sections around
> > ->cpu_exec_interrupt, so that the BQL critical section covers just the
> > call to ->cpu_exec_interrupt. Document which fields are now covered by
> > cpu_mutex.
> >
> > 4/5) same as 1/2 for ->cpu_exec_interrupt
> >
> > Patches 1/2 would be pretty large, but they're trivial to review just by
> > grepping for "->do_interrupt\s*=", and likewise for 4/5.
> >
>
> Thanks for the details !
>
> It seems like we will have 3 separate patches for this series, 1/2, 3, and
> 4/5.
>
> We will go in this direction.
>
> Thanks,
> -Rob
>
> > Thanks,
> >
> > Paolo
> >
Re: [PATCH v1 04/21] target/avr: add BQL to do_interrupt and cpu_exec_interrupt
On Thu, 6 Aug 2020 at 14:37, Michael Rolnik wrote:
>
> Hi Robert.
>
> I am sorry but how can I apply it? following this what I get
>
> error: patch failed: accel/tcg/cpu-exec.c:558
> error: accel/tcg/cpu-exec.c: patch does not apply
> error: patch failed: target/arm/helper.c:9808
> error: target/arm/helper.c: patch does not apply
> error: patch failed: target/ppc/excp_helper.c:1056
> error: target/ppc/excp_helper.c: patch does not apply
> error: patch failed: target/sh4/helper.c:62
> error: target/sh4/helper.c: patch does not apply
> error: patch failed: target/unicore32/softmmu.c:118
> error: target/unicore32/softmmu.c: patch does not apply
>
Hi Michael,
This patch is based on the per-cpu locks patch series:
https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05314.html
Our current WIP tree for this interrupts patch is here:
https://github.com/rf972/qemu/commits/int_core_v1.4
Also, just so you know, based on the initial feedback we are going
to substantially change this series.
Another version will be sent out in a few days.
Thanks & Regards,
-Rob
>
>
> On Wed, Aug 5, 2020 at 9:17 PM Robert Foley wrote:
>>
>> This is part of a series of changes to remove the implied BQL
>> from the common code of cpu_handle_interrupt and
>> cpu_handle_exception. As part of removing the implied BQL
>> from the common code, we are pushing the BQL holding
>> down into the per-arch implementation functions of
>> do_interrupt and cpu_exec_interrupt.
>>
>> The purpose of this set of changes is to set the groundwork
>> so that an arch could move towards removing
>> the BQL from the cpu_handle_interrupt/exception paths.
>>
>> This approach was suggested by Paolo Bonzini.
>> For reference, here are two key posts in the discussion, explaining
>> the reasoning/benefits of this approach.
>> https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
>> https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
>>
>> Signed-off-by: Robert Foley
>> ---
>> target/avr/helper.c | 12 +++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/avr/helper.c b/target/avr/helper.c
>> index d96d14372b..f0d625c195 100644
>> --- a/target/avr/helper.c
>> +++ b/target/avr/helper.c
>> @@ -30,6 +30,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int
>> interrupt_request)
>> CPUClass *cc = CPU_GET_CLASS(cs);
>> AVRCPU *cpu = AVR_CPU(cs);
>> CPUAVRState *env = >env;
>> +qemu_mutex_lock_iothread();
>>
>> if (interrupt_request & CPU_INTERRUPT_RESET) {
>> if (cpu_interrupts_enabled(env)) {
>> @@ -53,6 +54,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int
>> interrupt_request)
>> ret = true;
>> }
>> }
>> +qemu_mutex_unlock_iothread();
>> return ret;
>> }
>>
>> @@ -61,10 +63,15 @@ void avr_cpu_do_interrupt(CPUState *cs)
>> AVRCPU *cpu = AVR_CPU(cs);
>> CPUAVRState *env = >env;
>>
>> -uint32_t ret = env->pc_w;
>> +uint32_t ret;
>> int vector = 0;
>> int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1;
>> int base = 0;
>> +bool bql = !qemu_mutex_iothread_locked();
>> +if (bql) {
>> +qemu_mutex_lock_iothread();
>> +}
>> +ret = env->pc_w;
>>
>> if (cs->exception_index == EXCP_RESET) {
>> vector = 0;
>> @@ -87,6 +94,9 @@ void avr_cpu_do_interrupt(CPUState *cs)
>> env->sregI = 0; /* clear Global Interrupt Flag */
>>
>> cs->exception_index = -1;
>> +if (bql) {
>> +qemu_mutex_unlock_iothread();
>> +}
>> }
>>
>> int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,
>> --
>> 2.17.1
>>
>
>
> --
> Best Regards,
> Michael Rolnik
Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
On Thu, 6 Aug 2020 at 05:22, Paolo Bonzini wrote:
>
> On 05/08/20 21:18, Richard Henderson wrote:
> > On 8/5/20 11:12 AM, Robert Foley wrote:
> >> This change removes the implied BQL from the cpu_handle_interrupt,
> >> and cpu_handle_exception paths. This BQL acquire is being pushed
> >> down into the per arch implementation.
> >>
> >> Signed-off-by: Robert Foley
> >> ---
> >> accel/tcg/cpu-exec.c | 19 +++
> >> 1 file changed, 11 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> >> index 80d0e649b2..8e2bfd97a1 100644
> >> --- a/accel/tcg/cpu-exec.c
> >> +++ b/accel/tcg/cpu-exec.c
> >> @@ -517,9 +517,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;
> >>
> >
> > This patch is not bisectable. The removal of the lock here needs to happen
> > at
> > the end, or something.
>
> Indeed the series should be structured like this:
>
> 1) rename all *_do_interrupt functions to *_do_interrupt_locked
>
> 2) add back *_do_interrupt that takes the BQL and calls
> *_do_interrupt_locked, point ->do_interrupt to it, remove the BQL from
> cpu-exec.c
>
> 3) modify the cpu_mutex and BQL critical sections around
> ->cpu_exec_interrupt, so that the BQL critical section covers just the
> call to ->cpu_exec_interrupt. Document which fields are now covered by
> cpu_mutex.
>
> 4/5) same as 1/2 for ->cpu_exec_interrupt
>
> Patches 1/2 would be pretty large, but they're trivial to review just by
> grepping for "->do_interrupt\s*=", and likewise for 4/5.
>
Thanks for the details !
It seems like we will have 3 separate patches for this series, 1/2, 3, and 4/5.
We will go in this direction.
Thanks,
-Rob
> Thanks,
>
> Paolo
>
Re: [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt
On Wed, 5 Aug 2020 at 15:18, Richard Henderson
wrote:
>
> On 8/5/20 11:12 AM, Robert Foley wrote:
> > @@ -299,8 +299,12 @@ void alpha_cpu_do_interrupt(CPUState *cs)
> > {
> > AlphaCPU *cpu = ALPHA_CPU(cs);
> > CPUAlphaState *env = >env;
> > -int i = cs->exception_index;
> > -
> > +int i;
> > +bool bql = !qemu_mutex_iothread_locked();
> > +if (bql) {
> > +qemu_mutex_lock_iothread();
> > +}
>
> Why does this patch for alpha need to check qemu_mutex_iothread_locked and the
> next patch for arm does not?
>
In alpha (and arm) the do_interrupt function can be called separately or by
cpu_exec_interrupt. In the case where do_interrupt gets called separately
it needs to take the BQL (bql == true).
In the case where cpu_exec_interrupt is holding the BQL, and calls do_interrupt,
do_interrupt needs to check qemu_mutex_iothread_locked, and in this case not get
the lock (bql == false).
The next patch for arm, checks qemu_mutex_iothread_locked in its do_interrupt
function, but not in its cpu_exec_interrupt function, the same pattern
as for alpha.
Thanks & Regards,
-Rob
>
> r~
[PATCH v1 21/21] target/xtensa: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/xtensa/exc_helper.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/xtensa/exc_helper.c b/target/xtensa/exc_helper.c
index 01d1e56feb..fd33a56847 100644
--- a/target/xtensa/exc_helper.c
+++ b/target/xtensa/exc_helper.c
@@ -200,6 +200,7 @@ void xtensa_cpu_do_interrupt(CPUState *cs)
XtensaCPU *cpu = XTENSA_CPU(cs);
CPUXtensaState *env = >env;
+qemu_mutex_lock_iothread();
if (cs->exception_index == EXC_IRQ) {
qemu_log_mask(CPU_LOG_INT,
"%s(EXC_IRQ) level = %d, cintlevel = %d, "
@@ -252,6 +253,7 @@ void xtensa_cpu_do_interrupt(CPUState *cs)
break;
}
check_interrupts(env);
+qemu_mutex_unlock_iothread();
}
#else
void xtensa_cpu_do_interrupt(CPUState *cs)
--
2.17.1
[PATCH v1 16/21] target/rx: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/rx/helper.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/rx/helper.c b/target/rx/helper.c
index a6a337a311..a456b727ed 100644
--- a/target/rx/helper.c
+++ b/target/rx/helper.c
@@ -48,6 +48,10 @@ void rx_cpu_do_interrupt(CPUState *cs)
CPURXState *env = >env;
int do_irq = cs->interrupt_request & INT_FLAGS;
uint32_t save_psw;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
env->in_sleep = 0;
@@ -117,6 +121,9 @@ void rx_cpu_do_interrupt(CPUState *cs)
(vec & 0xff), expname);
}
env->regs[0] = env->isp;
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool rx_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
@@ -124,6 +131,7 @@ bool rx_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
RXCPU *cpu = RXCPU(cs);
CPURXState *env = >env;
int accept = 0;
+qemu_mutex_lock_iothread();
/* hardware interrupt (Normal) */
if ((interrupt_request & CPU_INTERRUPT_HARD) &&
env->psw_i && (env->psw_ipl < env->req_ipl)) {
@@ -138,8 +146,10 @@ bool rx_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
}
if (accept) {
rx_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 17/21] target/s390x: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/s390x/excp_helper.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index dde7afc2f0..b215b4a4a7 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -470,7 +470,10 @@ void s390_cpu_do_interrupt(CPUState *cs)
S390CPU *cpu = S390_CPU(cs);
CPUS390XState *env = >env;
bool stopped = false;
-
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
qemu_log_mask(CPU_LOG_INT, "%s: %d at psw=%" PRIx64 ":%" PRIx64 "\n",
__func__, cs->exception_index, env->psw.mask, env->psw.addr);
@@ -541,10 +544,14 @@ try_deliver:
/* unhalt if we had a WAIT PSW somehwere in our injection chain */
s390_cpu_unhalt(cpu);
}
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_HARD) {
S390CPU *cpu = S390_CPU(cs);
CPUS390XState *env = >env;
@@ -552,10 +559,12 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
if (env->ex_value) {
/* Execution of the target insn is indivisible from
the parent EXECUTE insn. */
+qemu_mutex_unlock_iothread();
return false;
}
if (s390_cpu_has_int(cpu)) {
s390_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
if (env->psw.mask & PSW_MASK_WAIT) {
@@ -564,6 +573,7 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HALT);
}
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 08/21] target/lm32: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/lm32/helper.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/lm32/helper.c b/target/lm32/helper.c
index 1130fc8884..4439b06ecc 100644
--- a/target/lm32/helper.c
+++ b/target/lm32/helper.c
@@ -152,6 +152,10 @@ void lm32_cpu_do_interrupt(CPUState *cs)
{
LM32CPU *cpu = LM32_CPU(cs);
CPULM32State *env = >env;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
qemu_log_mask(CPU_LOG_INT,
"exception at pc=%x type=%x\n", env->pc, cs->exception_index);
@@ -196,18 +200,24 @@ void lm32_cpu_do_interrupt(CPUState *cs)
cs->exception_index);
break;
}
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool lm32_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
LM32CPU *cpu = LM32_CPU(cs);
CPULM32State *env = >env;
+qemu_mutex_lock_iothread();
if ((interrupt_request & CPU_INTERRUPT_HARD) && (env->ie & IE_IE)) {
cs->exception_index = EXCP_IRQ;
lm32_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 14/21] target/ppc: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/ppc/excp_helper.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index bf9e1e27e9..4530230d65 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -870,7 +870,9 @@ void ppc_cpu_do_interrupt(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
+qemu_mutex_lock_iothread();
powerpc_excp(cpu, env->excp_model, cs->exception_index);
+qemu_mutex_unlock_iothread();
}
static void ppc_hw_interrupt(CPUPPCState *env)
@@ -1056,14 +1058,17 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
{
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_HARD) {
ppc_hw_interrupt(env);
if (env->pending_interrupts == 0) {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 20/21] target/unicore32: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/unicore32/helper.c | 3 +++
target/unicore32/softmmu.c | 7 +++
2 files changed, 10 insertions(+)
diff --git a/target/unicore32/helper.c b/target/unicore32/helper.c
index 54c26871fe..d79284d224 100644
--- a/target/unicore32/helper.c
+++ b/target/unicore32/helper.c
@@ -169,6 +169,7 @@ void helper_cp1_putc(target_ulong regval)
bool uc32_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_HARD) {
UniCore32CPU *cpu = UNICORE32_CPU(cs);
CPUUniCore32State *env = >env;
@@ -176,8 +177,10 @@ bool uc32_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
if (!(env->uncached_asr & ASR_I)) {
cs->exception_index = UC32_EXCP_INTR;
uc32_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
}
+qemu_mutex_unlock_iothread();
return false;
}
diff --git a/target/unicore32/softmmu.c b/target/unicore32/softmmu.c
index 9660bd2a27..ca9b92aad0 100644
--- a/target/unicore32/softmmu.c
+++ b/target/unicore32/softmmu.c
@@ -81,6 +81,10 @@ void uc32_cpu_do_interrupt(CPUState *cs)
CPUUniCore32State *env = >env;
uint32_t addr;
int new_mode;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
switch (cs->exception_index) {
case UC32_EXCP_PRIV:
@@ -118,6 +122,9 @@ void uc32_cpu_do_interrupt(CPUState *cs)
env->regs[30] = env->regs[31];
env->regs[31] = addr;
cpu_interrupt_request_or(cs, CPU_INTERRUPT_EXITTB);
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
static int get_phys_addr_ucv2(CPUUniCore32State *env, uint32_t address,
--
2.17.1
[PATCH v1 18/21] target/sh4: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/sh4/helper.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 1e32365c75..c4d5b9a374 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -62,8 +62,11 @@ void superh_cpu_do_interrupt(CPUState *cs)
{
SuperHCPU *cpu = SUPERH_CPU(cs);
CPUSH4State *env = >env;
-int do_irq = cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
-int do_exp, irq_vector = cs->exception_index;
+int do_irq;
+int do_exp, irq_vector;
+qemu_mutex_lock_iothread();
+do_irq = cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
+irq_vector = cs->exception_index;
/* prioritize exceptions over interrupts */
@@ -79,9 +82,11 @@ void superh_cpu_do_interrupt(CPUState *cs)
should be loaded with the kernel entry point.
qemu_system_reset_request takes care of that. */
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+qemu_mutex_unlock_iothread();
return;
}
if (do_irq && !env->in_sleep) {
+qemu_mutex_unlock_iothread();
return; /* masked */
}
}
@@ -91,6 +96,7 @@ void superh_cpu_do_interrupt(CPUState *cs)
irq_vector = sh_intc_get_pending_vector(env->intc_handle,
(env->sr >> 4) & 0xf);
if (irq_vector == -1) {
+qemu_mutex_unlock_iothread();
return; /* masked */
}
}
@@ -180,14 +186,17 @@ void superh_cpu_do_interrupt(CPUState *cs)
env->pc = env->vbr + 0x100;
break;
}
+qemu_mutex_unlock_iothread();
return;
}
if (do_irq) {
env->intevt = irq_vector;
env->pc = env->vbr + 0x600;
+qemu_mutex_unlock_iothread();
return;
}
+qemu_mutex_unlock_iothread();
}
static void update_itlb_use(CPUSH4State * env, int itlbnb)
--
2.17.1
[PATCH v1 19/21] target/sparc: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/sparc/cpu.c | 3 +++
target/sparc/int32_helper.c | 13 -
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 20c7c0c434..13b5a038e8 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -79,6 +79,7 @@ static void sparc_cpu_reset(DeviceState *dev)
static bool sparc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_HARD) {
SPARCCPU *cpu = SPARC_CPU(cs);
CPUSPARCState *env = >env;
@@ -90,10 +91,12 @@ static bool sparc_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
if (type != TT_EXTINT || cpu_pil_allowed(env, pil)) {
cs->exception_index = env->interrupt_index;
sparc_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
}
}
+qemu_mutex_unlock_iothread();
return false;
}
diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
index 9a71e1abd8..3940e945ed 100644
--- a/target/sparc/int32_helper.c
+++ b/target/sparc/int32_helper.c
@@ -69,7 +69,12 @@ void sparc_cpu_do_interrupt(CPUState *cs)
{
SPARCCPU *cpu = SPARC_CPU(cs);
CPUSPARCState *env = >env;
-int cwp, intno = cs->exception_index;
+int cwp, intno;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
+intno = cs->exception_index;
/* Compute PSR before exposing state. */
if (env->cc_op != CC_OP_FLAGS) {
@@ -115,6 +120,9 @@ void sparc_cpu_do_interrupt(CPUState *cs)
"Error state",
cs->exception_index, excp_name_str(cs->exception_index));
}
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
return;
}
#endif
@@ -136,6 +144,9 @@ void sparc_cpu_do_interrupt(CPUState *cs)
env->qemu_irq_ack(env, env->irq_manager, intno);
}
#endif
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
#if !defined(CONFIG_USER_ONLY)
--
2.17.1
[PATCH v1 07/21] target/i386: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/i386/seg_helper.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
index 818f65f35f..114d4a0d24 100644
--- a/target/i386/seg_helper.c
+++ b/target/i386/seg_helper.c
@@ -1284,7 +1284,7 @@ void x86_cpu_do_interrupt(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = >env;
-
+qemu_mutex_lock_iothread();
#if defined(CONFIG_USER_ONLY)
/* if user mode only, we simulate a fake exception
which will be handled outside the cpu execution
@@ -1308,6 +1308,7 @@ void x86_cpu_do_interrupt(CPUState *cs)
env->old_exception = -1;
}
#endif
+qemu_mutex_unlock_iothread();
}
void do_interrupt_x86_hardirq(CPUX86State *env, int intno, int is_hw)
@@ -1320,9 +1321,10 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = >env;
int intno;
-
+qemu_mutex_lock_iothread();
interrupt_request = x86_cpu_pending_interrupt(cs, interrupt_request);
if (!interrupt_request) {
+qemu_mutex_unlock_iothread();
return false;
}
@@ -1377,6 +1379,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
}
/* Ensure that no TB jump will be modified as the program flow was
changed. */
+qemu_mutex_unlock_iothread();
return true;
}
--
2.17.1
[PATCH v1 13/21] target/openrisc: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/openrisc/interrupt.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index 3eab771dcd..361f242954 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -28,6 +28,10 @@
void openrisc_cpu_do_interrupt(CPUState *cs)
{
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
#ifndef CONFIG_USER_ONLY
OpenRISCCPU *cpu = OPENRISC_CPU(cs);
CPUOpenRISCState *env = >env;
@@ -99,6 +103,9 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
#endif
cs->exception_index = -1;
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool openrisc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
@@ -106,6 +113,7 @@ bool openrisc_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
OpenRISCCPU *cpu = OPENRISC_CPU(cs);
CPUOpenRISCState *env = >env;
int idx = -1;
+qemu_mutex_lock_iothread();
if ((interrupt_request & CPU_INTERRUPT_HARD) && (env->sr & SR_IEE)) {
idx = EXCP_INT;
@@ -116,7 +124,9 @@ bool openrisc_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
if (idx >= 0) {
cs->exception_index = idx;
openrisc_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 05/21] target/cris: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/cris/helper.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/target/cris/helper.c b/target/cris/helper.c
index 67946d9246..22aecde0f5 100644
--- a/target/cris/helper.c
+++ b/target/cris/helper.c
@@ -45,8 +45,10 @@ void cris_cpu_do_interrupt(CPUState *cs)
CRISCPU *cpu = CRIS_CPU(cs);
CPUCRISState *env = >env;
+qemu_mutex_lock_iothread();
cs->exception_index = -1;
env->pregs[PR_ERP] = env->pc;
+qemu_mutex_unlock_iothread();
}
void crisv10_cpu_do_interrupt(CPUState *cs)
@@ -128,6 +130,10 @@ void crisv10_cpu_do_interrupt(CPUState *cs)
CRISCPU *cpu = CRIS_CPU(cs);
CPUCRISState *env = >env;
int ex_vec = -1;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
D_LOG("exception index=%d interrupt_req=%d\n",
cs->exception_index,
@@ -183,6 +189,9 @@ void crisv10_cpu_do_interrupt(CPUState *cs)
env->pregs[PR_CCS],
env->pregs[PR_PID],
env->pregs[PR_ERP]);
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
void cris_cpu_do_interrupt(CPUState *cs)
@@ -190,6 +199,10 @@ void cris_cpu_do_interrupt(CPUState *cs)
CRISCPU *cpu = CRIS_CPU(cs);
CPUCRISState *env = >env;
int ex_vec = -1;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
D_LOG("exception index=%d interrupt_req=%d\n",
cs->exception_index,
@@ -265,6 +278,9 @@ void cris_cpu_do_interrupt(CPUState *cs)
env->pregs[PR_CCS],
env->pregs[PR_PID],
env->pregs[PR_ERP]);
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
hwaddr cris_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
@@ -294,6 +310,7 @@ bool cris_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
CRISCPU *cpu = CRIS_CPU(cs);
CPUCRISState *env = >env;
bool ret = false;
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_HARD
&& (env->pregs[PR_CCS] & I_FLAG)
@@ -315,6 +332,7 @@ bool cris_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
ret = true;
}
}
+qemu_mutex_unlock_iothread();
return ret;
}
--
2.17.1
[PATCH v1 15/21] target/riscv: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/riscv/cpu_helper.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 75d2ae3434..5050802e95 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -80,14 +80,17 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
{
#if !defined(CONFIG_USER_ONLY)
if (interrupt_request & CPU_INTERRUPT_HARD) {
+qemu_mutex_lock_iothread();
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = >env;
int interruptno = riscv_cpu_local_irq_pending(env);
if (interruptno >= 0) {
cs->exception_index = RISCV_EXCP_INT_FLAG | interruptno;
riscv_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
}
#endif
return false;
@@ -822,6 +825,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
*/
void riscv_cpu_do_interrupt(CPUState *cs)
{
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
#if !defined(CONFIG_USER_ONLY)
RISCVCPU *cpu = RISCV_CPU(cs);
@@ -982,4 +989,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
#endif
cs->exception_index = EXCP_NONE; /* mark handled to qemu */
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
--
2.17.1
[PATCH v1 09/21] target/m68k: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/m68k/op_helper.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 4a032a150e..0c476a 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -448,7 +448,9 @@ void m68k_cpu_do_interrupt(CPUState *cs)
M68kCPU *cpu = M68K_CPU(cs);
CPUM68KState *env = >env;
+qemu_mutex_lock_iothread();
do_interrupt_all(env, 0);
+qemu_mutex_unlock_iothread();
}
static inline void do_interrupt_m68k_hardirq(CPUM68KState *env)
@@ -508,6 +510,7 @@ bool m68k_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
{
M68kCPU *cpu = M68K_CPU(cs);
CPUM68KState *env = >env;
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_HARD
&& ((env->sr & SR_I) >> SR_I_SHIFT) < env->pending_level) {
@@ -519,8 +522,10 @@ bool m68k_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
*/
cs->exception_index = env->pending_vector;
do_interrupt_m68k_hardirq(env);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 11/21] target/mips: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/mips/helper.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/mips/helper.c b/target/mips/helper.c
index afd78b1990..6595d18702 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -1085,6 +1085,10 @@ static inline void set_badinstr_registers(CPUMIPSState
*env)
void mips_cpu_do_interrupt(CPUState *cs)
{
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
#if !defined(CONFIG_USER_ONLY)
MIPSCPU *cpu = MIPS_CPU(cs);
CPUMIPSState *env = >env;
@@ -1396,10 +1400,14 @@ void mips_cpu_do_interrupt(CPUState *cs)
}
#endif
cs->exception_index = EXCP_NONE;
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_HARD) {
MIPSCPU *cpu = MIPS_CPU(cs);
CPUMIPSState *env = >env;
@@ -1410,9 +1418,11 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
cs->exception_index = EXCP_EXT_INTERRUPT;
env->error_code = 0;
mips_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 12/21] target/nios2: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/nios2/cpu.c| 3 +++
target/nios2/helper.c | 8 +++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index fe5fd9adfd..fd05406eac 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -102,13 +102,16 @@ static bool nios2_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
{
Nios2CPU *cpu = NIOS2_CPU(cs);
CPUNios2State *env = >env;
+qemu_mutex_lock_iothread();
if ((interrupt_request & CPU_INTERRUPT_HARD) &&
(env->regs[CR_STATUS] & CR_STATUS_PIE)) {
cs->exception_index = EXCP_IRQ;
nios2_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
diff --git a/target/nios2/helper.c b/target/nios2/helper.c
index 57c97bde3c..46d53551d4 100644
--- a/target/nios2/helper.c
+++ b/target/nios2/helper.c
@@ -52,7 +52,10 @@ void nios2_cpu_do_interrupt(CPUState *cs)
{
Nios2CPU *cpu = NIOS2_CPU(cs);
CPUNios2State *env = >env;
-
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
switch (cs->exception_index) {
case EXCP_IRQ:
assert(env->regs[CR_STATUS] & CR_STATUS_PIE);
@@ -198,6 +201,9 @@ void nios2_cpu_do_interrupt(CPUState *cs)
cs->exception_index);
break;
}
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
hwaddr nios2_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
--
2.17.1
[PATCH v1 06/21] target/hppa: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/hppa/int_helper.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index 462747baf8..eda40bc5d9 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -94,12 +94,20 @@ void hppa_cpu_do_interrupt(CPUState *cs)
{
HPPACPU *cpu = HPPA_CPU(cs);
CPUHPPAState *env = >env;
-int i = cs->exception_index;
-target_ureg iaoq_f = env->iaoq_f;
-target_ureg iaoq_b = env->iaoq_b;
-uint64_t iasq_f = env->iasq_f;
-uint64_t iasq_b = env->iasq_b;
-
+int i;
+target_ureg iaoq_f;
+target_ureg iaoq_b;
+uint64_t iasq_f;
+uint64_t iasq_b;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
+i = cs->exception_index;
+iaoq_f = env->iaoq_f;
+iaoq_b = env->iaoq_b;
+iasq_f = env->iasq_f;
+iasq_b = env->iasq_b;
#ifndef CONFIG_USER_ONLY
target_ureg old_psw;
@@ -244,6 +252,9 @@ void hppa_cpu_do_interrupt(CPUState *cs)
env->cr[CR_IOR]));
}
cs->exception_index = -1;
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool hppa_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
@@ -251,6 +262,7 @@ bool hppa_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
#ifndef CONFIG_USER_ONLY
HPPACPU *cpu = HPPA_CPU(cs);
CPUHPPAState *env = >env;
+qemu_mutex_lock_iothread();
/* If interrupts are requested and enabled, raise them. */
if ((env->psw & PSW_I) && (interrupt_request & CPU_INTERRUPT_HARD)) {
@@ -258,6 +270,7 @@ bool hppa_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
hppa_cpu_do_interrupt(cs);
return true;
}
+qemu_mutex_unlock_iothread();
#endif
return false;
}
--
2.17.1
[PATCH v1 10/21] target/microblaze: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/microblaze/helper.c | 20
1 file changed, 20 insertions(+)
diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index ab2ceeb055..ae8ff2bea4 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -32,10 +32,17 @@ void mb_cpu_do_interrupt(CPUState *cs)
{
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = >env;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
cs->exception_index = -1;
env->res_addr = RES_ADDR_NONE;
env->regs[14] = env->sregs[SR_PC];
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
@@ -113,6 +120,10 @@ void mb_cpu_do_interrupt(CPUState *cs)
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = >env;
uint32_t t;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
/* IMM flag cannot propagate across a branch and into the dslot. */
assert(!((env->iflags & D_FLAG) && (env->iflags & IMM_FLAG)));
@@ -123,6 +134,9 @@ void mb_cpu_do_interrupt(CPUState *cs)
case EXCP_HW_EXCP:
if (!(env->pvr.regs[0] & PVR0_USE_EXC_MASK)) {
qemu_log_mask(LOG_GUEST_ERROR, "Exception raised on system
without exceptions!\n");
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
return;
}
@@ -262,6 +276,9 @@ void mb_cpu_do_interrupt(CPUState *cs)
cs->exception_index);
break;
}
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
hwaddr mb_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
@@ -291,6 +308,7 @@ bool mb_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
{
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = >env;
+qemu_mutex_lock_iothread();
if ((interrupt_request & CPU_INTERRUPT_HARD)
&& (env->sregs[SR_MSR] & MSR_IE)
@@ -298,7 +316,9 @@ bool mb_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
&& !(env->iflags & (D_FLAG | IMM_FLAG))) {
cs->exception_index = EXCP_IRQ;
mb_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 03/21] target/arm: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/arm/cpu.c| 13 ++---
target/arm/helper.c | 17 -
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 401832ea95..b8544f0f0a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -528,12 +528,17 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
{
CPUClass *cc = CPU_GET_CLASS(cs);
CPUARMState *env = cs->env_ptr;
-uint32_t cur_el = arm_current_el(env);
-bool secure = arm_is_secure(env);
-uint64_t hcr_el2 = arm_hcr_el2_eff(env);
+uint32_t cur_el;
+bool secure;
+uint64_t hcr_el2;
uint32_t target_el;
uint32_t excp_idx;
+qemu_mutex_lock_iothread();
+cur_el = arm_current_el(env);
+secure = arm_is_secure(env);
+hcr_el2 = arm_hcr_el2_eff(env);
+
/* The prioritization of interrupts is IMPLEMENTATION DEFINED. */
if (interrupt_request & CPU_INTERRUPT_FIQ) {
@@ -568,12 +573,14 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
goto found;
}
}
+qemu_mutex_unlock_iothread();
return false;
found:
cs->exception_index = excp_idx;
env->exception.target_el = target_el;
cc->do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c5ea2c25ea..3a22d40598 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9759,7 +9759,13 @@ void arm_cpu_do_interrupt(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = >env;
-unsigned int new_el = env->exception.target_el;
+unsigned int new_el;
+
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
+new_el = env->exception.target_el;
assert(!arm_feature(env, ARM_FEATURE_M));
@@ -9776,6 +9782,9 @@ void arm_cpu_do_interrupt(CPUState *cs)
if (arm_is_psci_call(cpu, cs->exception_index)) {
arm_handle_psci_call(cpu);
qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
return;
}
@@ -9787,6 +9796,9 @@ void arm_cpu_do_interrupt(CPUState *cs)
#ifdef CONFIG_TCG
if (cs->exception_index == EXCP_SEMIHOST) {
handle_semihosting(cs);
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
return;
}
#endif
@@ -9808,6 +9820,9 @@ void arm_cpu_do_interrupt(CPUState *cs)
if (!kvm_enabled()) {
cpu_interrupt_request_or(cs, CPU_INTERRUPT_EXITTB);
}
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
#endif /* !CONFIG_USER_ONLY */
--
2.17.1
[PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
This change removes the implied BQL from the cpu_handle_interrupt,
and cpu_handle_exception paths. This BQL acquire is being pushed
down into the per arch implementation.
Signed-off-by: Robert Foley
---
accel/tcg/cpu-exec.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 80d0e649b2..8e2bfd97a1 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -517,9 +517,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;
if (unlikely(cpu->singlestep_enabled)) {
@@ -558,7 +556,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
if (unlikely(cpu_interrupt_request(cpu))) {
int interrupt_request;
-qemu_mutex_lock_iothread();
+cpu_mutex_lock(cpu);
interrupt_request = cpu_interrupt_request(cpu);
if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
/* Mask out external interrupts for this step. */
@@ -567,7 +565,7 @@ 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();
+cpu_mutex_unlock(cpu);
return true;
}
if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
@@ -577,13 +575,15 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
cpu_halted_set(cpu, 1);
cpu->exception_index = EXCP_HLT;
-qemu_mutex_unlock_iothread();
+cpu_mutex_unlock(cpu);
return true;
}
#if defined(TARGET_I386)
else if (interrupt_request & CPU_INTERRUPT_INIT) {
X86CPU *x86_cpu = X86_CPU(cpu);
CPUArchState *env = _cpu->env;
+cpu_mutex_unlock(cpu);
+qemu_mutex_lock_iothread();
replay_interrupt();
cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
do_cpu_init(x86_cpu);
@@ -595,7 +595,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
else if (interrupt_request & CPU_INTERRUPT_RESET) {
replay_interrupt();
cpu_reset(cpu);
-qemu_mutex_unlock_iothread();
+cpu_mutex_unlock(cpu);
return true;
}
#endif
@@ -604,7 +604,9 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
True when it is, and we should restart on a new TB,
and via longjmp via cpu_loop_exit. */
else {
+cpu_mutex_unlock(cpu);
if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
+cpu_mutex_lock(cpu);
replay_interrupt();
/*
* After processing the interrupt, ensure an EXCP_DEBUG is
@@ -614,6 +616,8 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
cpu->exception_index =
(cpu->singlestep_enabled ? EXCP_DEBUG : -1);
*last_tb = NULL;
+} else {
+cpu_mutex_lock(cpu);
}
/* The target hook may have updated the 'cpu->interrupt_request';
* reload the 'interrupt_request' value */
@@ -627,7 +631,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
}
/* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */
-qemu_mutex_unlock_iothread();
+cpu_mutex_unlock(cpu);
}
/* Finally, check if we need to exit to the main loop. */
@@ -691,7 +695,6 @@ static inline void cpu_loop_exec_tb(CPUState *cpu,
TranslationBlock *tb,
}
#endif
}
-
/* main execution loop */
int cpu_exec(CPUState *cpu)
--
2.17.1
[PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path
The purpose of this change is to set the groundwork so that an arch could move towards removing the BQL from the cpu_handle_interrupt/exception paths. The BQL is a bottleneck in scaling to more cores. And this cpu_handle_interrupt/exception path is one of the key BQL users as measured by the QEMU sync profiling (qsp). We have chosen to break up the process of removing BQL from this path into two pieces: 1) Changes to the core/common functions of cpu_handle_interrupt/exception to drop the holding of the BQL. The holding of the BQL is pushed down to the per-arch implementation code. This set of changes is handled in this patch. This approach of pushing the BQL down to the per arch functions was suggested by Paolo Bonzini. For reference, here are two key posts in the discussion, explaining the reasoning/benefits of this approach. https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html 2) Removing the BQL from the per-arch functions. Since the arch now has the code that grabs the BQL, each arch can change its use of the BQL for interrupts independently. We leave it up to the arch to make the change at the time that makes sense. It is worth mentioning that we are working on per-arch changes in line with 2), and plan to submit these. In other words, we plan to set the groundwork with this patch series and then will take advantage of it in later series. This patch series is based on the per-CPU locks patch: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05314.html Robert Foley (21): accel/tcg: Change interrupt/exception handling to remove implied BQL target/alpha: add BQL to do_interrupt and cpu_exec_interrupt target/arm: add BQL to do_interrupt and cpu_exec_interrupt target/avr: add BQL to do_interrupt and cpu_exec_interrupt target/cris: add BQL to do_interrupt and cpu_exec_interrupt target/hppa: add BQL to do_interrupt and cpu_exec_interrupt target/i386: add BQL to do_interrupt and cpu_exec_interrupt target/lm32: add BQL to do_interrupt and cpu_exec_interrupt target/m68k: add BQL to do_interrupt and cpu_exec_interrupt target/microblaze: add BQL to do_interrupt and cpu_exec_interrupt target/mips: add BQL to do_interrupt and cpu_exec_interrupt target/nios2: add BQL to do_interrupt and cpu_exec_interrupt target/openrisc: add BQL to do_interrupt and cpu_exec_interrupt target/ppc: add BQL to do_interrupt and cpu_exec_interrupt target/riscv: add BQL to do_interrupt and cpu_exec_interrupt target/rx: add BQL to do_interrupt and cpu_exec_interrupt target/s390x: add BQL to do_interrupt and cpu_exec_interrupt target/sh4: add BQL to do_interrupt and cpu_exec_interrupt target/sparc: add BQL to do_interrupt and cpu_exec_interrupt target/unicore32: add BQL to do_interrupt and cpu_exec_interrupt target/xtensa: add BQL to do_interrupt and cpu_exec_interrupt accel/tcg/cpu-exec.c| 19 +++ target/alpha/helper.c | 15 +-- target/arm/cpu.c| 13 ++--- target/arm/helper.c | 17 - target/avr/helper.c | 12 +++- target/cris/helper.c| 18 ++ target/hppa/int_helper.c| 25 +++-- target/i386/seg_helper.c| 7 +-- target/lm32/helper.c| 10 ++ target/m68k/op_helper.c | 5 + target/microblaze/helper.c | 20 target/mips/helper.c| 10 ++ target/nios2/cpu.c | 3 +++ target/nios2/helper.c | 8 +++- target/openrisc/interrupt.c | 10 ++ target/ppc/excp_helper.c| 5 + target/riscv/cpu_helper.c | 10 ++ target/rx/helper.c | 10 ++ target/s390x/excp_helper.c | 12 +++- target/sh4/helper.c | 13 +++-- target/sparc/cpu.c | 3 +++ target/sparc/int32_helper.c | 13 - target/unicore32/helper.c | 3 +++ target/unicore32/softmmu.c | 7 +++ target/xtensa/exc_helper.c | 2 ++ 25 files changed, 242 insertions(+), 28 deletions(-) -- 2.17.1
[PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/alpha/helper.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 55d7274d94..18169ae1c5 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -299,8 +299,12 @@ void alpha_cpu_do_interrupt(CPUState *cs)
{
AlphaCPU *cpu = ALPHA_CPU(cs);
CPUAlphaState *env = >env;
-int i = cs->exception_index;
-
+int i;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
+i = cs->exception_index;
if (qemu_loglevel_mask(CPU_LOG_INT)) {
static int count;
const char *name = "";
@@ -405,6 +409,9 @@ void alpha_cpu_do_interrupt(CPUState *cs)
/* Switch to PALmode. */
env->flags |= ENV_FLAG_PAL_MODE;
#endif /* !USER_ONLY */
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
bool alpha_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
@@ -412,9 +419,11 @@ bool alpha_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
AlphaCPU *cpu = ALPHA_CPU(cs);
CPUAlphaState *env = >env;
int idx = -1;
+qemu_mutex_lock_iothread();
/* We never take interrupts while in PALmode. */
if (env->flags & ENV_FLAG_PAL_MODE) {
+qemu_mutex_unlock_iothread();
return false;
}
@@ -446,8 +455,10 @@ bool alpha_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
cs->exception_index = idx;
env->error_code = 0;
alpha_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
return true;
}
+qemu_mutex_unlock_iothread();
return false;
}
--
2.17.1
[PATCH v1 04/21] target/avr: add BQL to do_interrupt and cpu_exec_interrupt
This is part of a series of changes to remove the implied BQL
from the common code of cpu_handle_interrupt and
cpu_handle_exception. As part of removing the implied BQL
from the common code, we are pushing the BQL holding
down into the per-arch implementation functions of
do_interrupt and cpu_exec_interrupt.
The purpose of this set of changes is to set the groundwork
so that an arch could move towards removing
the BQL from the cpu_handle_interrupt/exception paths.
This approach was suggested by Paolo Bonzini.
For reference, here are two key posts in the discussion, explaining
the reasoning/benefits of this approach.
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html
Signed-off-by: Robert Foley
---
target/avr/helper.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index d96d14372b..f0d625c195 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -30,6 +30,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
CPUClass *cc = CPU_GET_CLASS(cs);
AVRCPU *cpu = AVR_CPU(cs);
CPUAVRState *env = >env;
+qemu_mutex_lock_iothread();
if (interrupt_request & CPU_INTERRUPT_RESET) {
if (cpu_interrupts_enabled(env)) {
@@ -53,6 +54,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
ret = true;
}
}
+qemu_mutex_unlock_iothread();
return ret;
}
@@ -61,10 +63,15 @@ void avr_cpu_do_interrupt(CPUState *cs)
AVRCPU *cpu = AVR_CPU(cs);
CPUAVRState *env = >env;
-uint32_t ret = env->pc_w;
+uint32_t ret;
int vector = 0;
int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1;
int base = 0;
+bool bql = !qemu_mutex_iothread_locked();
+if (bql) {
+qemu_mutex_lock_iothread();
+}
+ret = env->pc_w;
if (cs->exception_index == EXCP_RESET) {
vector = 0;
@@ -87,6 +94,9 @@ void avr_cpu_do_interrupt(CPUState *cs)
env->sregI = 0; /* clear Global Interrupt Flag */
cs->exception_index = -1;
+if (bql) {
+qemu_mutex_unlock_iothread();
+}
}
int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,
--
2.17.1
Re: [PATCH 2/2] accel/tcg: interrupt/exception handling uses bql_interrupt flag
On Fri, 31 Jul 2020 at 14:02, Paolo Bonzini wrote:
>
> On 31/07/20 14:51, Robert Foley wrote:
> > This change removes the implied BQL from the cpu_handle_interrupt,
> > and cpu_handle_exception paths. We can now select per-arch if
> > the BQL is needed or not by using the bql_interrupt flag.
> > By default, the core code holds the BQL.
> > One benefit of this change is that it leaves it up to the arch
> > to make the change to remove BQL when it makes sense.
> >
> > Signed-off-by: Robert Foley
>
> No, please just modify all implementation to do lock/unlock. It's a
> simpler patch than this on.
Sure, we will update the patch based on this.
To clarify, the suggestion here is to remove the bql_interrupt flag
that we added and change all the per-arch interrupt callback code to
do the lock/unlock of the BQL? So for example change
x86_cpu_exec_interrupt, and arm_cpu_exec_interrupt, etc to lock/unlock BQL?
Thanks,
-Rob
>
> Paolo
>
> > ---
> > accel/tcg/cpu-exec.c | 34 ++
> > 1 file changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > index 80d0e649b2..cde27ee0bf 100644
> > --- a/accel/tcg/cpu-exec.c
> > +++ b/accel/tcg/cpu-exec.c
> > @@ -517,9 +517,13 @@ static inline bool cpu_handle_exception(CPUState *cpu,
> > int *ret)
> > #else
> > if (replay_exception()) {
> > CPUClass *cc = CPU_GET_CLASS(cpu);
> > -qemu_mutex_lock_iothread();
> > +if (cc->bql_interrupt) {
> > +qemu_mutex_lock_iothread();
> > +}
> > cc->do_interrupt(cpu);
> > -qemu_mutex_unlock_iothread();
> > +if (cc->bql_interrupt) {
> > +qemu_mutex_unlock_iothread();
> > +}
> > cpu->exception_index = -1;
> >
> > if (unlikely(cpu->singlestep_enabled)) {
> > @@ -558,7 +562,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> > if (unlikely(cpu_interrupt_request(cpu))) {
> > int interrupt_request;
> >
> > -qemu_mutex_lock_iothread();
> > +cpu_mutex_lock(cpu);
> > interrupt_request = cpu_interrupt_request(cpu);
> > if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
> > /* Mask out external interrupts for this step. */
> > @@ -567,7 +571,7 @@ 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();
> > +cpu_mutex_unlock(cpu);
> > return true;
> > }
> > if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
> > @@ -577,13 +581,15 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> > cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
> > cpu_halted_set(cpu, 1);
> > cpu->exception_index = EXCP_HLT;
> > -qemu_mutex_unlock_iothread();
> > +cpu_mutex_unlock(cpu);
> > return true;
> > }
> > #if defined(TARGET_I386)
> > else if (interrupt_request & CPU_INTERRUPT_INIT) {
> > X86CPU *x86_cpu = X86_CPU(cpu);
> > CPUArchState *env = _cpu->env;
> > +cpu_mutex_unlock(cpu);
> > +qemu_mutex_lock_iothread();
> > replay_interrupt();
> > cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
> > do_cpu_init(x86_cpu);
> > @@ -595,7 +601,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> > else if (interrupt_request & CPU_INTERRUPT_RESET) {
> > replay_interrupt();
> > cpu_reset(cpu);
> > -qemu_mutex_unlock_iothread();
> > +cpu_mutex_unlock(cpu);
> > return true;
> > }
> > #endif
> > @@ -604,7 +610,15 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> > True when it is, and we should restart on a new TB,
> > and via longjmp via cpu_loop_exit. */
> > else {
> > +cpu_mutex_unlock(cpu);
> > +if (cc->bql_interrupt) {
> > +qemu_mutex_lock_iothread();
> > +}
> > if (cc->cpu_exec_interr
Re: [PATCH 1/2] hw/core: Add bql_interrupt flag to CPUClass
On Fri, 31 Jul 2020 at 13:44, Eduardo Habkost wrote:
> >
> > +static inline void cpu_class_disable_bql_interrupt(CPUClass *cc)
> > +{
> > +cc->bql_interrupt = false;
> > +}
>
> Class data is not supposed to change outside class_init. Why do
> you need this function? I don't see it being used anywhere in
> this series.
This function was to be called from changes in a later patch series
that depend on these changes. BTW, I added a correction above,
it should be disable, not enable. The idea is that it is initialized to true,
but then the per arch changes would use this call at init time to set
it to false
as needed.
We can remove this function from this series and add it in later when
it gets used,
it might make things more clear.
Thanks,
-Rob
> --
> Eduardo
>
[PATCH 1/2] hw/core: Add bql_interrupt flag to CPUClass
The new flag bql_interrupt, allows the CPUClass to
determine if the BQL should be held during calls to
cpu_exec_interrupt or do_interrupt.
This is being added in preparation for changes in
cpu_handle_interrupt, which will use this flag.
Signed-off-by: Robert Foley
---
hw/core/cpu.c | 1 +
include/hw/core/cpu.h | 8
2 files changed, 9 insertions(+)
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 8707ce2c34..7ab88caa97 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -425,6 +425,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
k->cpu_exec_exit = cpu_common_noop;
k->cpu_exec_interrupt = cpu_common_exec_interrupt;
k->adjust_watchpoint_address = cpu_adjust_watchpoint_address;
+k->bql_interrupt = true;
set_bit(DEVICE_CATEGORY_CPU, dc->categories);
dc->realize = cpu_common_realizefn;
dc->unrealize = cpu_common_unrealizefn;
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 6a2c77682f..d2c426ee5d 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -157,6 +157,8 @@ struct TranslationBlock;
* @disas_set_info: Setup architecture specific components of disassembly info
* @adjust_watchpoint_address: Perform a target-specific adjustment to an
* address before attempting to match it against watchpoints.
+ * @bql_interrupt: Hold BQL while performing the cpu_exec_interrupt
+ * or do_interrupt call.
*
* Represents a CPU family or model.
*/
@@ -227,6 +229,7 @@ typedef struct CPUClass {
/* Keep non-pointer data at the end to minimize holes. */
int gdb_num_core_regs;
bool gdb_stop_before_watchpoint;
+bool bql_interrupt;
} CPUClass;
/*
@@ -589,6 +592,11 @@ static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
}
}
+static inline void cpu_class_enable_bql_interrupt(CPUClass *cc)
+{
+cc->bql_interrupt = true;
+}
+
/**
* qemu_tcg_mttcg_enabled:
* Check whether we are running MultiThread TCG or not.
--
2.17.1
[PATCH 2/2] accel/tcg: interrupt/exception handling uses bql_interrupt flag
This change removes the implied BQL from the cpu_handle_interrupt,
and cpu_handle_exception paths. We can now select per-arch if
the BQL is needed or not by using the bql_interrupt flag.
By default, the core code holds the BQL.
One benefit of this change is that it leaves it up to the arch
to make the change to remove BQL when it makes sense.
Signed-off-by: Robert Foley
---
accel/tcg/cpu-exec.c | 34 ++
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 80d0e649b2..cde27ee0bf 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -517,9 +517,13 @@ static inline bool cpu_handle_exception(CPUState *cpu, int
*ret)
#else
if (replay_exception()) {
CPUClass *cc = CPU_GET_CLASS(cpu);
-qemu_mutex_lock_iothread();
+if (cc->bql_interrupt) {
+qemu_mutex_lock_iothread();
+}
cc->do_interrupt(cpu);
-qemu_mutex_unlock_iothread();
+if (cc->bql_interrupt) {
+qemu_mutex_unlock_iothread();
+}
cpu->exception_index = -1;
if (unlikely(cpu->singlestep_enabled)) {
@@ -558,7 +562,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
if (unlikely(cpu_interrupt_request(cpu))) {
int interrupt_request;
-qemu_mutex_lock_iothread();
+cpu_mutex_lock(cpu);
interrupt_request = cpu_interrupt_request(cpu);
if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
/* Mask out external interrupts for this step. */
@@ -567,7 +571,7 @@ 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();
+cpu_mutex_unlock(cpu);
return true;
}
if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
@@ -577,13 +581,15 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
cpu_halted_set(cpu, 1);
cpu->exception_index = EXCP_HLT;
-qemu_mutex_unlock_iothread();
+cpu_mutex_unlock(cpu);
return true;
}
#if defined(TARGET_I386)
else if (interrupt_request & CPU_INTERRUPT_INIT) {
X86CPU *x86_cpu = X86_CPU(cpu);
CPUArchState *env = _cpu->env;
+cpu_mutex_unlock(cpu);
+qemu_mutex_lock_iothread();
replay_interrupt();
cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
do_cpu_init(x86_cpu);
@@ -595,7 +601,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
else if (interrupt_request & CPU_INTERRUPT_RESET) {
replay_interrupt();
cpu_reset(cpu);
-qemu_mutex_unlock_iothread();
+cpu_mutex_unlock(cpu);
return true;
}
#endif
@@ -604,7 +610,15 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
True when it is, and we should restart on a new TB,
and via longjmp via cpu_loop_exit. */
else {
+cpu_mutex_unlock(cpu);
+if (cc->bql_interrupt) {
+qemu_mutex_lock_iothread();
+}
if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
+if (cc->bql_interrupt) {
+qemu_mutex_unlock_iothread();
+}
+cpu_mutex_lock(cpu);
replay_interrupt();
/*
* After processing the interrupt, ensure an EXCP_DEBUG is
@@ -614,6 +628,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
cpu->exception_index =
(cpu->singlestep_enabled ? EXCP_DEBUG : -1);
*last_tb = NULL;
+} else {
+if (cc->bql_interrupt) {
+qemu_mutex_unlock_iothread();
+}
+cpu_mutex_lock(cpu);
}
/* The target hook may have updated the 'cpu->interrupt_request';
* reload the 'interrupt_request' value */
@@ -627,7 +646,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
}
/* If we exit via cpu_loop_exit/longjmp it is reset in cpu_exec */
-qemu_mutex_unlock_iothread();
+cpu_mutex_unlock(cpu);
}
/* Finally, check if we need to exit to the main loop. */
@@ -691,7 +710,6 @@ static inline void cpu_loop_exec_tb(CPUState *cpu,
TranslationBlock *tb,
}
#endif
}
-
/* main execution loop */
int cpu_exec(CPUState *cpu)
--
2.17.1
[PATCH 0/2] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path
The purpose of this change is to set the groundwork so that an arch could move towards removing the BQL from the cpu_handle_interrupt/exception paths. The BQL is a bottlneck in scaling to more cores. And this cpu_handle_interrupt/exception path is one of the key BQL users as measured by the QEMU sync profiling (qsp). We have chosen to break up the process of removing BQL from this path into two pieces: 1) Changes to the core/common functions of cpu_handle_interrupt/exception allowing a per-arch decision to hold BQL. The common case and the default is for the core code to hold the BQL (bql=true). This set of changes is handled in this patch. 2) Removing the BQL from the per-arch functions. 1) makes it possible for an arch to set bql=false so that the common code does not hold the BQL across the cpu_handle_interrupt/exception call. This allows the arch to handle locking in this path We leave it up to the arch to make the change at the time that makes sense. It is worth mentioning that we are working on per-arch changes in line with 2), and plan to submit these. In other words, we plan to set the groundwork with this patch series and then will take advantage of it in later series. This patch series is based on the per-CPU locks patch: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05314.html Robert Foley (2): hw/core: Add bql_interrupt flag to CPUClass accel/tcg: interrupt/exception handling uses bql_interrupt flag accel/tcg/cpu-exec.c | 34 ++ hw/core/cpu.c | 1 + include/hw/core/cpu.h | 8 3 files changed, 35 insertions(+), 8 deletions(-) -- 2.17.1
Re: [PATCH v3 06/16] accel/tcg: better handle memory constrained systems
Reviewed-by: Robert Foley
On Fri, 24 Jul 2020 at 02:49, Alex Bennée wrote:
>
> It turns out there are some 64 bit systems that have relatively low
> amounts of physical memory available to them (typically CI system).
> Even with swapping available a 1GB translation buffer that fills up
> can put the machine under increased memory pressure. Detect these low
> memory situations and reduce tb_size appropriately.
>
> Fixes: 600e17b2615 ("accel/tcg: increase default code gen buffer size for 64
> bit")
> Signed-off-by: Alex Bennée
> Cc: BALATON Zoltan
> Cc: Christian Ehrhardt
>
> ---
> v3
> - use slightly tweak the logic to taper off more nicely
> ---
> accel/tcg/translate-all.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 2afa46bd2b1..2d83013633b 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -976,7 +976,12 @@ static inline size_t size_code_gen_buffer(size_t tb_size)
> {
> /* Size the buffer. */
> if (tb_size == 0) {
> -tb_size = DEFAULT_CODE_GEN_BUFFER_SIZE;
> +size_t phys_mem = qemu_get_host_physmem();
> +if (phys_mem == 0) {
> +tb_size = DEFAULT_CODE_GEN_BUFFER_SIZE;
> +} else {
> +tb_size = MIN(DEFAULT_CODE_GEN_BUFFER_SIZE, phys_mem / 8);
> +}
> }
> if (tb_size < MIN_CODE_GEN_BUFFER_SIZE) {
> tb_size = MIN_CODE_GEN_BUFFER_SIZE;
> --
> 2.20.1
>
>
Re: tests/vm infrastructure fails to notice that QEMU dying is a failure
On Fri, 17 Jul 2020 at 18:24, John Snow wrote: > > - The real problem, though: Why is QEMU hanging? It might need a longer > timeout, or it might be having problems with the console socket again. > > (CC Robert Foley who has been working on the Console socket draining > problems. Maybe he has some insight here?) When we did see the console issues we would see a hung stack like this: #0 0xd43d141c in qemu_chr_write_buffer #1 0xd43d194c in qemu_chr_write #2 0xd43d3968 in qemu_chr_fe_write_all #3 0xd417cf80 in pl011_write #4 0xd3f3c7b0 in memory_region_write_accessor #5 0xd3f3a1fc in access_with_adjusted_size #6 0xd3f3e828 in memory_region_dispatch_write #7 0xd3f517b0 in io_writex #8 0x574a1d34 in code_gen_buffer () #9 0xd3f67228 in cpu_tb_exec #10 0xd3f67228 in cpu_loop_exec_tb #11 0xd3f67228 in cpu_exec #12 0xd3f2dbe4 in tcg_cpu_exec #13 0xd3f305e8 in qemu_tcg_cpu_thread_fn #14 0xd4441d88 in qemu_thread_start #15 0x85bec088 in start_thread #16 0x85b5c4ec in thread_start However, since we added console socket draining thread, it seems to have fixed this and presently basevm.py should be using this console draining for the vm-build-openbsd. When QEMU is hanging and exceeding our shutdown timeout, could we (optionally) send something like a SIGABRT to QEMU to force a core dump so we can get the stack and see where QEMU is hung? I suppose that presumes it is reproducible, but it might help to remove doubt in cases where QEMU hangs. -Rob > > --js >
[PATCH v1 2/3] python/qemu: Change ConsoleSocket to optionally drain socket.
The primary purpose of this change is to clean up
machine.py's console_socket property to return a single type,
a ConsoleSocket.
ConsoleSocket now derives from a socket, which means that
in the default case (of not draining), machine.py
will see the same behavior as it did prior to ConsoleSocket.
Signed-off-by: Robert Foley
---
python/qemu/console_socket.py | 92 +--
python/qemu/machine.py| 13 ++---
2 files changed, 59 insertions(+), 46 deletions(-)
diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 09986bc215..70869fbbdc 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -13,68 +13,75 @@ which can drain a socket and optionally dump the bytes to
file.
# the COPYING file in the top-level directory.
#
-import asyncore
import socket
import threading
from collections import deque
import time
-class ConsoleSocket(asyncore.dispatcher):
+class ConsoleSocket(socket.socket):
"""
ConsoleSocket represents a socket attached to a char device.
-Drains the socket and places the bytes into an in memory buffer
-for later processing.
+Optionally (if drain==True), drains the socket and places the bytes
+into an in memory buffer for later processing.
Optionally a file path can be passed in and we will also
dump the characters to this file for debugging purposes.
"""
-def __init__(self, address, file=None):
+def __init__(self, address, file=None, drain=False):
self._recv_timeout_sec = 300
self._sleep_time = 0.5
self._buffer = deque()
-self._asyncore_thread = None
-self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
-self._sock.connect(address)
+socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM)
+self.connect(address)
self._logfile = None
if file:
self._logfile = open(file, "w")
-asyncore.dispatcher.__init__(self, sock=self._sock)
self._open = True
-self._thread_start()
+if drain:
+self._drain_thread = self._thread_start()
+else:
+self._drain_thread = None
-def _thread_start(self):
-"""Kick off a thread to wait on the asyncore.loop"""
-if self._asyncore_thread is not None:
-return
-self._asyncore_thread = threading.Thread(target=asyncore.loop,
- kwargs={'timeout':1})
-self._asyncore_thread.daemon = True
-self._asyncore_thread.start()
+def _drain_fn(self):
+"""Drains the socket and runs while the socket is open."""
+while self._open:
+try:
+self._drain_socket()
+except socket.timeout:
+# The socket is expected to timeout since we set a
+# short timeout to allow the thread to exit when
+# self._open is set to False.
+time.sleep(self._sleep_time)
-def handle_close(self):
-"""redirect close to base class"""
-# Call the base class close, but not self.close() since
-# handle_close() occurs in the context of the thread which
-# self.close() attempts to join.
-asyncore.dispatcher.close(self)
+def _thread_start(self):
+"""Kick off a thread to drain the socket."""
+# Configure socket to not block and timeout.
+# This allows our drain thread to not block
+# on recieve and exit smoothly.
+socket.socket.setblocking(self, False)
+socket.socket.settimeout(self, 1)
+drain_thread = threading.Thread(target=self._drain_fn)
+drain_thread.daemon = True
+drain_thread.start()
+return drain_thread
def close(self):
"""Close the base object and wait for the thread to terminate"""
if self._open:
self._open = False
-asyncore.dispatcher.close(self)
-if self._asyncore_thread is not None:
-thread, self._asyncore_thread = self._asyncore_thread, None
+if self._drain_thread is not None:
+thread, self._drain_thread = self._drain_thread, None
thread.join()
+socket.socket.close(self)
if self._logfile:
self._logfile.close()
self._logfile = None
-def handle_read(self):
+def _drain_socket(self):
"""process arriving characters into in memory _buffer"""
-data = asyncore.dispatcher.recv(self, 1)
+data = socket.socket.recv(self, 1)
# latin1 is needed since there are some chars
# we are receiving that cannot be enco
[PATCH v1 3/3] tests/vm: add shutdown timeout in basevm.py
We are adding the shutdown timeout to solve an issue
we now see where the aarch64 VMs timeout on shutdown
under TCG.
There is a new 3 second timeout in machine.py,
which we override in basevm.py when shutting down.
Signed-off-by: Robert Foley
---
tests/vm/basevm.py | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 7acb48b876..3fac20e929 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -80,6 +80,8 @@ class BaseVM(object):
arch = "#arch"
# command to halt the guest, can be overridden by subclasses
poweroff = "poweroff"
+# Time to wait for shutdown to finish.
+shutdown_timeout_default = 30
# enable IPv6 networking
ipv6 = True
# This is the timeout on the wait for console bytes.
@@ -87,7 +89,7 @@ class BaseVM(object):
# Scale up some timeouts under TCG.
# 4 is arbitrary, but greater than 2,
# since we found we need to wait more than twice as long.
-tcg_ssh_timeout_multiplier = 4
+tcg_timeout_multiplier = 4
def __init__(self, args, config=None):
self._guest = None
self._genisoimage = args.genisoimage
@@ -141,9 +143,12 @@ class BaseVM(object):
if args.jobs and args.jobs > 1:
self._args += ["-smp", "%d" % args.jobs]
if kvm_available(self.arch):
+self._shutdown_timeout = self.shutdown_timeout_default
self._args += ["-enable-kvm"]
else:
logging.info("KVM not available, not using -enable-kvm")
+self._shutdown_timeout = \
+self.shutdown_timeout_default * self.tcg_timeout_multiplier
self._data_args = []
if self._config['qemu_args'] != None:
@@ -423,7 +428,7 @@ class BaseVM(object):
def wait_ssh(self, wait_root=False, seconds=300, cmd="exit 0"):
# Allow more time for VM to boot under TCG.
if not kvm_available(self.arch):
-seconds *= self.tcg_ssh_timeout_multiplier
+seconds *= self.tcg_timeout_multiplier
starttime = datetime.datetime.now()
endtime = starttime + datetime.timedelta(seconds=seconds)
cmd_success = False
@@ -441,14 +446,14 @@ class BaseVM(object):
raise Exception("Timeout while waiting for guest ssh")
def shutdown(self):
-self._guest.shutdown()
+self._guest.shutdown(timeout=self._shutdown_timeout)
def wait(self):
-self._guest.wait()
+self._guest.wait(timeout=self._shutdown_timeout)
def graceful_shutdown(self):
self.ssh_root(self.poweroff)
-self._guest.wait()
+self._guest.wait(timeout=self._shutdown_timeout)
def qmp(self, *args, **kwargs):
return self._guest.qmp(*args, **kwargs)
--
2.17.1
[PATCH v1 1/3] python/qemu: Cleanup changes to ConsoleSocket
The changes to console_socket.py and machine.py are to
cleanup for pylint and flake8.
Reviewed-by: Alex Bennée
Signed-off-by: Robert Foley
---
python/qemu/console_socket.py | 57 ++-
python/qemu/machine.py| 7 +++--
python/qemu/pylintrc | 2 +-
3 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 830cb7c628..09986bc215 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -1,12 +1,9 @@
-#!/usr/bin/env python3
-#
-# This python module implements a ConsoleSocket object which is
-# designed always drain the socket itself, and place
-# the bytes into a in memory buffer for later processing.
-#
-# Optionally a file path can be passed in and we will also
-# dump the characters to this file for debug.
-#
+"""
+QEMU Console Socket Module:
+
+This python module implements a ConsoleSocket object,
+which can drain a socket and optionally dump the bytes to file.
+"""
# Copyright 2020 Linaro
#
# Authors:
@@ -15,20 +12,27 @@
# This code is licensed under the GPL version 2 or later. See
# the COPYING file in the top-level directory.
#
+
import asyncore
import socket
import threading
-import io
-import os
-import sys
from collections import deque
import time
-import traceback
+
class ConsoleSocket(asyncore.dispatcher):
+"""
+ConsoleSocket represents a socket attached to a char device.
+Drains the socket and places the bytes into an in memory buffer
+for later processing.
+
+Optionally a file path can be passed in and we will also
+dump the characters to this file for debugging purposes.
+"""
def __init__(self, address, file=None):
self._recv_timeout_sec = 300
+self._sleep_time = 0.5
self._buffer = deque()
self._asyncore_thread = None
self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
@@ -70,31 +74,28 @@ class ConsoleSocket(asyncore.dispatcher):
def handle_read(self):
"""process arriving characters into in memory _buffer"""
-try:
-data = asyncore.dispatcher.recv(self, 1)
-# latin1 is needed since there are some chars
-# we are receiving that cannot be encoded to utf-8
-# such as 0xe2, 0x80, 0xA6.
-string = data.decode("latin1")
-except:
-print("Exception seen.")
-traceback.print_exc()
-return
+data = asyncore.dispatcher.recv(self, 1)
+# latin1 is needed since there are some chars
+# we are receiving that cannot be encoded to utf-8
+# such as 0xe2, 0x80, 0xA6.
+string = data.decode("latin1")
if self._logfile:
self._logfile.write("{}".format(string))
self._logfile.flush()
for c in string:
self._buffer.extend(c)
-def recv(self, n=1, sleep_delay_s=0.1):
-"""Return chars from in memory buffer"""
+def recv(self, buffer_size=1):
+"""Return chars from in memory buffer.
+ Maintains the same API as socket.socket.recv.
+"""
start_time = time.time()
-while len(self._buffer) < n:
-time.sleep(sleep_delay_s)
+while len(self._buffer) < buffer_size:
+time.sleep(self._sleep_time)
elapsed_sec = time.time() - start_time
if elapsed_sec > self._recv_timeout_sec:
raise socket.timeout
-chars = ''.join([self._buffer.popleft() for i in range(n)])
+chars = ''.join([self._buffer.popleft() for i in range(buffer_size)])
# We choose to use latin1 to remain consistent with
# handle_read() and give back the same data as the user would
# receive if they were reading directly from the
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 80c4d4a8b6..9956360a79 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -27,7 +27,7 @@ import socket
import tempfile
from typing import Optional, Type
from types import TracebackType
-from qemu.console_socket import ConsoleSocket
+from . import console_socket
from . import qmp
@@ -674,8 +674,9 @@ class QEMUMachine:
"""
if self._console_socket is None:
if self._drain_console:
-self._console_socket = ConsoleSocket(self._console_address,
-
file=self._console_log_path)
+self._console_socket = console_socket.ConsoleSocket(
+self._console_address,
+file=self._console_log_path)
else:
self._console_socket = socket.socket(socket.AF_UNI
[PATCH v1 0/3] python/qemu: follow-up changes for ConsoleSocket
For v1, we added a few minor changes, and also added one new patch in tests/vm to add a shutdown timeout. This fixes an issue we saw in testing the aarch64 VMs with TCG. This patch series introduces a few follow-up changes after the introduction of ConsoleSocket. The first patch introduces cleanup changes for pylint and flake8. The second patch allows machine.py to use a single type for the console_socket, a ConsoleSocket. Since machine.py will use ConsoleSocket for both the draining and non-draining cases, we changed ConsoleSocket to handle the case where it does not drain the socket at all and essentially behaves like a socket. Robert Foley (3): python/qemu: Cleanup changes to ConsoleSocket python/qemu: Change ConsoleSocket to optionally drain socket. tests/vm: add shutdown timeout in basevm.py python/qemu/console_socket.py | 137 +++--- python/qemu/machine.py| 14 ++-- python/qemu/pylintrc | 2 +- tests/vm/basevm.py| 15 ++-- 4 files changed, 94 insertions(+), 74 deletions(-) -- 2.17.1
Re: [PATCH 2/2] python/qemu: Change ConsoleSocket to optionally drain socket.
On Thu, 16 Jul 2020 at 09:42, Alex Bennée wrote: > > > +self._drain_thread = None > > +socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM) > > +self.connect(address) > > +self._drain = drain > > We end up with two variables that represent the fact we have draining > happening. Could we rationalise it into: > > if drain: > self._drain_thread = self._thread_start() > else > self._drain_thread = None # if this is needed > > And then tests for: > > if not self._drain: > > become > > if self._drain_thread is None: Good point, this is simpler. Will update. > > +if self._drain and self._drain_thread is not None: > > +thread, self._drain_thread = self._drain_thread, None > Would self._drain ever not have self._drain_thread set? No, I believe that if drain is set, it results in _drain_thread also being set. This will be cleaned up once we drop the _drain. > > > thread.join() > > +socket.socket.close(self) > > > diff --git a/python/qemu/machine.py b/python/qemu/machine.py > > index 6769359766..62709d86e4 100644 > > --- a/python/qemu/machine.py > > +++ b/python/qemu/machine.py > > @@ -22,7 +22,6 @@ import logging > > import os > > import subprocess > > import shutil > > -import socket > > FYI minor patch conflict here with master OK, will rebase and fix this conflict. Thanks & Regards, -Rob > > > import tempfile > > from typing import Optional, Type > > from types import TracebackType > > @@ -591,12 +590,8 @@ class QEMUMachine: > > Returns a socket connected to the console > > """ > > if self._console_socket is None: > > -if self._drain_console: > > -self._console_socket = console_socket.ConsoleSocket( > > -self._console_address, > > -file=self._console_log_path) > > -else: > > -self._console_socket = socket.socket(socket.AF_UNIX, > > - socket.SOCK_STREAM) > > -self._console_socket.connect(self._console_address) > > +self._console_socket = console_socket.ConsoleSocket( > > +self._console_address, > > +file=self._console_log_path, > > +drain=self._drain_console) > > return self._console_socket > > > -- > Alex Bennée
Re: [PATCH 1/2] python/qemu: Cleanup changes to ConsoleSocket
On Thu, 16 Jul 2020 at 07:07, Alex Bennée wrote: > > > Robert Foley writes: > > > The changes to console_socket.py and machine.py are to > > cleanup for pylint and flake8. > > > > Signed-off-by: Robert Foley > > --- > > python/qemu/console_socket.py | 58 +-- > > python/qemu/machine.py| 7 +++-- > > python/qemu/pylintrc | 2 +- > > 3 files changed, 34 insertions(+), 33 deletions(-) > > > > diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py > > index 830cb7c628..6a746c1dbf 100644 > > --- a/python/qemu/console_socket.py > > +++ b/python/qemu/console_socket.py > > @@ -1,12 +1,9 @@ > > > @@ -103,7 +104,6 @@ class ConsoleSocket(asyncore.dispatcher): > > > > def set_blocking(self): > > """Maintain compatibility with socket API""" > > -pass > > Hmm shouldn't this be with the change in 2/2 because I thought you > needed a "pass" for an empty function in python? Thanks for the review ! Sure, I can move this change to 2/2. Probably makes more sense there since we're changing this function there too. This change was one of the suggestions from John Snow. He pointed out that the pass is not needed here since the docstring takes the role of the function body. Thanks & Regards, -Rob > > Otherwise: > > Reviewed-by: Alex Bennée > > -- > Alex Bennée
[PATCH 2/2] python/qemu: Change ConsoleSocket to optionally drain socket.
The primary purpose of this change is to clean up
machine.py's console_socket property to return a single type,
a ConsoleSocket.
ConsoleSocket now derives from a socket, which means that
in the default case (of not draining), machine.py
will see the same behavior as it did prior to ConsoleSocket.
Signed-off-by: Robert Foley
---
python/qemu/console_socket.py | 81 +--
python/qemu/machine.py| 13 ++
2 files changed, 54 insertions(+), 40 deletions(-)
diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 6a746c1dbf..475de5b101 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -13,68 +13,76 @@ which can drain a socket and optionally dump the bytes to
file.
# the COPYING file in the top-level directory.
#
-import asyncore
import socket
import threading
from collections import deque
import time
-class ConsoleSocket(asyncore.dispatcher):
+class ConsoleSocket(socket.socket):
"""
ConsoleSocket represents a socket attached to a char device.
-Drains the socket and places the bytes into an in memory buffer
-for later processing.
+Optionally (if drain==True), drains the socket and places the bytes
+into an in memory buffer for later processing.
Optionally a file path can be passed in and we will also
dump the characters to this file for debugging purposes.
"""
-def __init__(self, address, file=None):
+def __init__(self, address, file=None, drain=False):
self._recv_timeout_sec = 300
self._sleep_time = 0.5
self._buffer = deque()
-self._asyncore_thread = None
-self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
-self._sock.connect(address)
+self._drain_thread = None
+socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM)
+self.connect(address)
+self._drain = drain
self._logfile = None
if file:
self._logfile = open(file, "w")
-asyncore.dispatcher.__init__(self, sock=self._sock)
self._open = True
-self._thread_start()
+if drain:
+self._thread_start()
+
+def _drain_fn(self):
+"""Drains the socket and runs while the socket is open."""
+while self._open:
+try:
+self._drain_socket()
+except socket.timeout:
+# The socket is expected to timeout since we set a
+# short timeout to allow the thread to exit when
+# self._open is set to False.
+time.sleep(self._sleep_time)
def _thread_start(self):
-"""Kick off a thread to wait on the asyncore.loop"""
-if self._asyncore_thread is not None:
+"""Kick off a thread to drain the socket."""
+if self._drain_thread is not None:
return
-self._asyncore_thread = threading.Thread(target=asyncore.loop,
- kwargs={'timeout':1})
-self._asyncore_thread.daemon = True
-self._asyncore_thread.start()
-
-def handle_close(self):
-"""redirect close to base class"""
-# Call the base class close, but not self.close() since
-# handle_close() occurs in the context of the thread which
-# self.close() attempts to join.
-asyncore.dispatcher.close(self)
+# Configure socket to not block and timeout.
+# This allows our drain thread to not block
+# on recieve and exit smoothly.
+socket.socket.setblocking(self, 0)
+socket.socket.settimeout(self, 1)
+self._drain_thread = threading.Thread(target=self._drain_fn)
+self._drain_thread.daemon = True
+self._drain_thread.start()
def close(self):
"""Close the base object and wait for the thread to terminate"""
if self._open:
self._open = False
-asyncore.dispatcher.close(self)
-if self._asyncore_thread is not None:
-thread, self._asyncore_thread = self._asyncore_thread, None
+if self._drain and self._drain_thread is not None:
+thread, self._drain_thread = self._drain_thread, None
thread.join()
+socket.socket.close(self)
if self._logfile:
self._logfile.close()
self._logfile = None
-def handle_read(self):
+def _drain_socket(self):
"""process arriving characters into in memory _buffer"""
-data = asyncore.dispatcher.recv(self, 1)
+data = socket.socket.recv(self, 1)
# latin1 is needed since there are some chars
# we are receiving that cannot b
[PATCH 1/2] python/qemu: Cleanup changes to ConsoleSocket
The changes to console_socket.py and machine.py are to
cleanup for pylint and flake8.
Signed-off-by: Robert Foley
---
python/qemu/console_socket.py | 58 +--
python/qemu/machine.py| 7 +++--
python/qemu/pylintrc | 2 +-
3 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 830cb7c628..6a746c1dbf 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -1,12 +1,9 @@
-#!/usr/bin/env python3
-#
-# This python module implements a ConsoleSocket object which is
-# designed always drain the socket itself, and place
-# the bytes into a in memory buffer for later processing.
-#
-# Optionally a file path can be passed in and we will also
-# dump the characters to this file for debug.
-#
+"""
+QEMU Console Socket Module:
+
+This python module implements a ConsoleSocket object,
+which can drain a socket and optionally dump the bytes to file.
+"""
# Copyright 2020 Linaro
#
# Authors:
@@ -15,20 +12,27 @@
# This code is licensed under the GPL version 2 or later. See
# the COPYING file in the top-level directory.
#
+
import asyncore
import socket
import threading
-import io
-import os
-import sys
from collections import deque
import time
-import traceback
+
class ConsoleSocket(asyncore.dispatcher):
+"""
+ConsoleSocket represents a socket attached to a char device.
+Drains the socket and places the bytes into an in memory buffer
+for later processing.
+
+Optionally a file path can be passed in and we will also
+dump the characters to this file for debugging purposes.
+"""
def __init__(self, address, file=None):
self._recv_timeout_sec = 300
+self._sleep_time = 0.5
self._buffer = deque()
self._asyncore_thread = None
self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
@@ -70,31 +74,28 @@ class ConsoleSocket(asyncore.dispatcher):
def handle_read(self):
"""process arriving characters into in memory _buffer"""
-try:
-data = asyncore.dispatcher.recv(self, 1)
-# latin1 is needed since there are some chars
-# we are receiving that cannot be encoded to utf-8
-# such as 0xe2, 0x80, 0xA6.
-string = data.decode("latin1")
-except:
-print("Exception seen.")
-traceback.print_exc()
-return
+data = asyncore.dispatcher.recv(self, 1)
+# latin1 is needed since there are some chars
+# we are receiving that cannot be encoded to utf-8
+# such as 0xe2, 0x80, 0xA6.
+string = data.decode("latin1")
if self._logfile:
self._logfile.write("{}".format(string))
self._logfile.flush()
for c in string:
self._buffer.extend(c)
-def recv(self, n=1, sleep_delay_s=0.1):
-"""Return chars from in memory buffer"""
+def recv(self, buffer_size=1):
+"""Return chars from in memory buffer.
+ Maintains the same API as socket.socket.recv.
+"""
start_time = time.time()
-while len(self._buffer) < n:
-time.sleep(sleep_delay_s)
+while len(self._buffer) < buffer_size:
+time.sleep(self._sleep_time)
elapsed_sec = time.time() - start_time
if elapsed_sec > self._recv_timeout_sec:
raise socket.timeout
-chars = ''.join([self._buffer.popleft() for i in range(n)])
+chars = ''.join([self._buffer.popleft() for i in range(buffer_size)])
# We choose to use latin1 to remain consistent with
# handle_read() and give back the same data as the user would
# receive if they were reading directly from the
@@ -103,7 +104,6 @@ class ConsoleSocket(asyncore.dispatcher):
def set_blocking(self):
"""Maintain compatibility with socket API"""
-pass
def settimeout(self, seconds):
"""Set current timeout on recv"""
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c25f0b42cf..6769359766 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -26,7 +26,7 @@ import socket
import tempfile
from typing import Optional, Type
from types import TracebackType
-from qemu.console_socket import ConsoleSocket
+from . import console_socket
from . import qmp
@@ -592,8 +592,9 @@ class QEMUMachine:
"""
if self._console_socket is None:
if self._drain_console:
-self._console_socket = ConsoleSocket(self._console_address,
-
file=s
[PATCH 0/2] python/qemu: follow-up changes for ConsoleSocket
This patch series introduces a few follow-up changes after the introduction of ConsoleSocket. The first patch introduces cleanup changes for pylint and flake8. The second patch allows machine.py to use a single type for the console_socket, a ConsoleSocket. Since machine.py will use ConsoleSocket for both the draining and non-draining cases, we changed ConsoleSocket to handle the case where it does not drain the socket at all and essentially behaves like a socket. Robert Foley (2): python/qemu: Cleanup changes to ConsoleSocket python/qemu: Change ConsoleSocket to optionally drain socket. python/qemu/console_socket.py | 133 +++--- python/qemu/machine.py| 14 ++-- python/qemu/pylintrc | 2 +- 3 files changed, 82 insertions(+), 67 deletions(-) -- 2.17.1
Re: [PULL 10/41] python/qemu: Add ConsoleSocket for optional use in QEMUMachine
Hi,
Thanks for the detailed feedback! I will look at making these changes.
On Fri, 10 Jul 2020 at 15:20, John Snow wrote:
>
>
>
> On 7/7/20 3:08 AM, Alex Bennée wrote:
> > From: Robert Foley
> >
>
> > +def recv(self, n=1, sleep_delay_s=0.1):
> > +"""Return chars from in memory buffer"""
> > +start_time = time.time()
> > +while len(self._buffer) < n:
> > +time.sleep(sleep_delay_s)
> > +elapsed_sec = time.time() - start_time
> > +if elapsed_sec > self._recv_timeout_sec:
> > +raise socket.timeout
> > +chars = ''.join([self._buffer.popleft() for i in range(n)])
> > +# We choose to use latin1 to remain consistent with
> > +# handle_read() and give back the same data as the user would
> > +# receive if they were reading directly from the
> > +# socket w/o our intervention.
> > +return chars.encode("latin1")
> > +
>
> console_socket.py:89:4: W0221: Parameters differ from overridden 'recv'
> method (arguments-differ)
>
> Seems pretty different from the asyncore.dispatcher recv method, is that
> intentional?
The intention is that the API be the same as asyncore.dispatcher recv.
The sleep_delay_s can be removed, and n is the same as buffer_size in
asyncore.dispatcher recv. Will plan to rename n -> buffer_size.
> https://github.com/python/cpython/blob/master/Lib/asyncore.py
>
> > def __enter__(self):
> > return self
> > @@ -580,7 +591,11 @@ class QEMUMachine:
> > Returns a socket connected to the console
> > """
> > if self._console_socket is None:
> > -self._console_socket = socket.socket(socket.AF_UNIX,
> > - socket.SOCK_STREAM)
> > -self._console_socket.connect(self._console_address)
> > +if self._drain_console:
> > +self._console_socket = ConsoleSocket(self._console_address,
> > +
> > file=self._console_log_path)
>
> Needs one more space, but the line is already too long as-is.
>
> > +else:
> > +self._console_socket = socket.socket(socket.AF_UNIX,
> > + socket.SOCK_STREAM)
> > +self._console_socket.connect(self._console_address)
> > return self._console_socket
> >
>
> This makes the typing for _console_socket really tough ... but
> technically not a regression as the mypy code isn't merged yet.
>From the comment on mypy, I understand that we need to return a
constant type?
One option to provide a constant type is to simply always return
ConsoleSocket here.
A few changes would be needed inside of ConsoleSocket,
but essentially ConsoleSocket would handle the detail
of draining the console (or not), and thus eliminate this
if/else above reducing it to something like this:
self._console_socket = ConsoleSocket(self._console_address,
file=self._console_log_path,
drain=self._drain_console)
How does this sound?
Thanks & Regards,
-Rob
>
> --js
>
Re: [PATCH v1 10/13] tests/plugins: add -Wno-unknown-warning-option to handle -Wpsabi
Reviewed-by: Robert Foley On Thu, 9 Jul 2020 at 10:13, Alex Bennée wrote: > > Not all compilers support the -Wpsabi (clang-9 in my case). > > Fixes: bac8d222a > Signed-off-by: Alex Bennée > --- > tests/plugin/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/plugin/Makefile b/tests/plugin/Makefile > index 0cb8e35ae407..dcfbd99b15b8 100644 > --- a/tests/plugin/Makefile > +++ b/tests/plugin/Makefile > @@ -18,7 +18,7 @@ NAMES += hwprofile > > SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES))) > > -QEMU_CFLAGS += -fPIC -Wpsabi > +QEMU_CFLAGS += -fPIC -Wno-unknown-warning-option -Wpsabi > QEMU_CFLAGS += -I$(SRC_PATH)/include/qemu > > all: $(SONAMES) > -- > 2.20.1 >
Re: [PATCH v1 08/13] plugins: expand the bb plugin to be thread safe and track per-cpu
Reviewed-by: Robert Foley
On Thu, 9 Jul 2020 at 10:13, Alex Bennée wrote:
>
> While there isn't any easy way to make the inline counts thread safe
> we can ensure the callback based ones are. While we are at it we can
> reduce introduce a new option ("idle") to dump a report of the current
> bb and insn count each time a vCPU enters the idle state.
>
> Signed-off-by: Alex Bennée
> Cc: Dave Bort
>
> ---
> v2
> - fixup for non-inline linux-user case
> - minor cleanup and re-factor
> ---
> tests/plugin/bb.c | 96 ---
> 1 file changed, 83 insertions(+), 13 deletions(-)
>
> diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
> index df19fd359df3..89c373e19cd8 100644
> --- a/tests/plugin/bb.c
> +++ b/tests/plugin/bb.c
> @@ -16,24 +16,67 @@
>
> QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>
> -static uint64_t bb_count;
> -static uint64_t insn_count;
> +typedef struct {
> +GMutex lock;
> +int index;
> +uint64_t bb_count;
> +uint64_t insn_count;
> +} CPUCount;
> +
> +/* Used by the inline & linux-user counts */
> static bool do_inline;
> +static CPUCount inline_count;
> +
> +/* Dump running CPU total on idle? */
> +static bool idle_report;
> +static GPtrArray *counts;
> +static int max_cpus;
> +
> +static void gen_one_cpu_report(CPUCount *count, GString *report)
> +{
> +if (count->bb_count) {
> +g_string_append_printf(report, "CPU%d: "
> + "bb's: %" PRIu64", insns: %" PRIu64 "\n",
> + count->index,
> + count->bb_count, count->insn_count);
> +}
> +}
>
> static void plugin_exit(qemu_plugin_id_t id, void *p)
> {
> -g_autofree gchar *out = g_strdup_printf(
> -"bb's: %" PRIu64", insns: %" PRIu64 "\n",
> -bb_count, insn_count);
> -qemu_plugin_outs(out);
> +g_autoptr(GString) report = g_string_new("");
> +
> +if (do_inline || !max_cpus) {
> +g_string_printf(report, "bb's: %" PRIu64", insns: %" PRIu64 "\n",
> +inline_count.bb_count, inline_count.insn_count);
> +} else {
> +g_ptr_array_foreach(counts, (GFunc) gen_one_cpu_report, report);
> +}
> +qemu_plugin_outs(report->str);
> +}
> +
> +static void vcpu_idle(qemu_plugin_id_t id, unsigned int cpu_index)
> +{
> +CPUCount *count = g_ptr_array_index(counts, cpu_index);
> +g_autoptr(GString) report = g_string_new("");
> +gen_one_cpu_report(count, report);
> +
> +if (report->len > 0) {
> +g_string_prepend(report, "Idling ");
> +qemu_plugin_outs(report->str);
> +}
> }
>
> static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
> {
> -unsigned long n_insns = (unsigned long)udata;
> +CPUCount *count = max_cpus ?
> +g_ptr_array_index(counts, cpu_index) : _count;
>
> -insn_count += n_insns;
> -bb_count++;
> +unsigned long n_insns = (unsigned long)udata;
> +g_mutex_lock(>lock);
> +count->insn_count += n_insns;
> +count->bb_count++;
> +g_mutex_unlock(>lock);
> }
>
> static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> @@ -42,9 +85,9 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
> qemu_plugin_tb *tb)
>
> if (do_inline) {
> qemu_plugin_register_vcpu_tb_exec_inline(tb,
> QEMU_PLUGIN_INLINE_ADD_U64,
> - _count, 1);
> + _count.bb_count, 1);
> qemu_plugin_register_vcpu_tb_exec_inline(tb,
> QEMU_PLUGIN_INLINE_ADD_U64,
> - _count, n_insns);
> + _count.insn_count,
> n_insns);
> } else {
> qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
> QEMU_PLUGIN_CB_NO_REGS,
> @@ -56,8 +99,35 @@ QEMU_PLUGIN_EXPORT int
> qemu_plugin_install(qemu_plugin_id_t id,
> const qemu_info_t *info,
> int argc, char **argv)
> {
> -if (argc && strcmp(argv[0], "inline") == 0) {
> -do_inline = true;
> +int i;
> +
> +for (i = 0; i < argc; i++) {
> +char *opt = argv[i];
> +if (g_strcmp0(opt, "inline") == 0) {
> +d
[PATCH] util/coroutine: Cleanup start_switch_fiber_ for TSAN.
This is a cleanup patch to follow-up the patch which introduced TSAN.
This patch makes separate start_switch_fiber_ functions for TSAN and ASAN.
This does two things:
1. Unrelated ASAN and TSAN code is separate and each function only
has arguments that are actually needed.
2. The co->tsan_caller_fiber and co->tsan_co_fiber fields are only
access from within #ifdef CONFIG_TSAN.
Signed-off-by: Robert Foley
---
util/coroutine-ucontext.c | 52 +--
1 file changed, 34 insertions(+), 18 deletions(-)
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 613f4c118e..bac164d1f1 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -47,8 +47,10 @@ typedef struct {
size_t stack_size;
sigjmp_buf env;
+#ifdef CONFIG_TSAN
void *tsan_co_fiber;
void *tsan_caller_fiber;
+#endif
#ifdef CONFIG_VALGRIND_H
unsigned int valgrind_stack_id;
@@ -72,7 +74,10 @@ union cc_arg {
int i[2];
};
-/* QEMU_ALWAYS_INLINE only does so if __OPTIMIZE__, so we cannot use it. */
+/*
+ * QEMU_ALWAYS_INLINE only does so if __OPTIMIZE__, so we cannot use it.
+ * always_inline is required to avoid TSan runtime fatal errors.
+ */
static inline __attribute__((always_inline))
void on_new_fiber(CoroutineUContext *co)
{
@@ -82,6 +87,7 @@ void on_new_fiber(CoroutineUContext *co)
#endif
}
+/* always_inline is required to avoid TSan runtime fatal errors. */
static inline __attribute__((always_inline))
void finish_switch_fiber(void *fake_stack_save)
{
@@ -104,18 +110,29 @@ void finish_switch_fiber(void *fake_stack_save)
#endif
}
-static inline __attribute__((always_inline)) void start_switch_fiber(
-CoroutineAction action, void **fake_stack_save,
-const void *bottom, size_t size, void *new_fiber)
+/* always_inline is required to avoid TSan runtime fatal errors. */
+static inline __attribute__((always_inline))
+void start_switch_fiber_asan(CoroutineAction action, void **fake_stack_save,
+ const void *bottom, size_t size)
{
#ifdef CONFIG_ASAN
__sanitizer_start_switch_fiber(
action == COROUTINE_TERMINATE ? NULL : fake_stack_save,
bottom, size);
#endif
+}
+
+/* always_inline is required to avoid TSan runtime fatal errors. */
+static inline __attribute__((always_inline))
+void start_switch_fiber_tsan(void **fake_stack_save,
+ CoroutineUContext *co,
+ bool caller)
+{
#ifdef CONFIG_TSAN
-void *curr_fiber =
-__tsan_get_current_fiber();
+void *new_fiber = caller ?
+ co->tsan_caller_fiber :
+ co->tsan_co_fiber;
+void *curr_fiber = __tsan_get_current_fiber();
__tsan_acquire(curr_fiber);
*fake_stack_save = curr_fiber;
@@ -139,12 +156,9 @@ static void coroutine_trampoline(int i0, int i1)
/* Initialize longjmp environment and switch back the caller */
if (!sigsetjmp(self->env, 0)) {
-start_switch_fiber(
-COROUTINE_YIELD,
-_stack_save,
-leader.stack,
-leader.stack_size,
-self->tsan_caller_fiber);
+start_switch_fiber_asan(COROUTINE_YIELD, _stack_save,
leader.stack,
+leader.stack_size);
+start_switch_fiber_tsan(_stack_save, self, true); /* true=caller
*/
siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
}
@@ -199,10 +213,10 @@ Coroutine *qemu_coroutine_new(void)
/* swapcontext() in, siglongjmp() back out */
if (!sigsetjmp(old_env, 0)) {
-start_switch_fiber(
-COROUTINE_YIELD,
-_stack_save,
-co->stack, co->stack_size, co->tsan_co_fiber);
+start_switch_fiber_asan(COROUTINE_YIELD, _stack_save, co->stack,
+co->stack_size);
+start_switch_fiber_tsan(_stack_save,
+co, false); /* false=not caller */
swapcontext(_uc, );
}
@@ -259,8 +273,10 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
ret = sigsetjmp(from->env, 0);
if (ret == 0) {
-start_switch_fiber(action, _stack_save,
- to->stack, to->stack_size, to->tsan_co_fiber);
+start_switch_fiber_asan(action, _stack_save, to->stack,
+to->stack_size);
+start_switch_fiber_tsan(_stack_save,
+to, false); /* false=not caller */
siglongjmp(to->env, action);
}
--
2.17.1
Re: [PATCH v1 11/18] tests/vm: switch from optsparse to argparse
Reviewed-by: Robert Foley
On Mon, 22 Jun 2020 at 10:41, Alex Bennée wrote:
>
> optparse has been deprecated since version 3.2 and argparse is the
> blessed replacement. Take the opportunity to enhance our help output
> showing defaults when called.
>
> Signed-off-by: Alex Bennée
> ---
> tests/vm/basevm.py | 84 +++---
> 1 file changed, 42 insertions(+), 42 deletions(-)
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 04d083409a5..93859362606 100644
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -23,7 +23,7 @@ from qemu.accel import kvm_available
> from qemu.machine import QEMUMachine
> import subprocess
> import hashlib
> -import optparse
> +import argparse
> import atexit
> import tempfile
> import shutil
> @@ -556,47 +556,47 @@ def parse_args(vmcls):
> else:
> return 1
>
> -parser = optparse.OptionParser(
> -description="VM test utility. Exit codes: "
> -"0 = success, "
> -"1 = command line error, "
> -"2 = environment initialization failed, "
> -"3 = test command failed")
> -parser.add_option("--debug", "-D", action="store_true",
> - help="enable debug output")
> -parser.add_option("--image", "-i", default="%s.img" % vmcls.name,
> - help="image file name")
> -parser.add_option("--force", "-f", action="store_true",
> - help="force build image even if image exists")
> -parser.add_option("--jobs", type=int, default=get_default_jobs(),
> - help="number of virtual CPUs")
> -parser.add_option("--verbose", "-V", action="store_true",
> - help="Pass V=1 to builds within the guest")
> -parser.add_option("--build-image", "-b", action="store_true",
> - help="build image")
> -parser.add_option("--build-qemu",
> - help="build QEMU from source in guest")
> -parser.add_option("--build-target",
> - help="QEMU build target", default="check")
> -parser.add_option("--build-path", default=None,
> - help="Path of build directory, "\
> - "for using build tree QEMU binary. ")
> -parser.add_option("--interactive", "-I", action="store_true",
> - help="Interactively run command")
> -parser.add_option("--snapshot", "-s", action="store_true",
> - help="run tests with a snapshot")
> -parser.add_option("--genisoimage", default="genisoimage",
> - help="iso imaging tool")
> -parser.add_option("--config", "-c", default=None,
> - help="Provide config yaml for configuration. "\
> - "See config_example.yaml for example.")
> -parser.add_option("--efi-aarch64",
> - default="/usr/share/qemu-efi-aarch64/QEMU_EFI.fd",
> - help="Path to efi image for aarch64 VMs.")
> -parser.add_option("--log-console", action="store_true",
> - help="Log console to file.")
> -parser.disable_interspersed_args()
> -return parser.parse_args()
> +parser = argparse.ArgumentParser(
> +formatter_class=argparse.ArgumentDefaultsHelpFormatter,
> +description="Utility for provisioning VMs and running builds",
> +epilog="""Remaining arguments are passed to the command.
> +Exit codes: 0 = success, 1 = command line error,
> +2 = environment initialization failed,
> +3 = test command failed""")
> +parser.add_argument("--debug", "-D", action="store_true",
> +help="enable debug output")
> +parser.add_argument("--image", "-i", default="%s.img" % vmcls.name,
> +help="image file name")
> +parser.add_argument("--force", "-f", action="store_true",
> +help="force bui
Re: [PATCH v1 12/18] tests/vm: allow us to take advantage of MTTCG
Reviewed-by: Robert Foley On Mon, 22 Jun 2020 at 10:38, Alex Bennée wrote: > > We currently limit TCG guests to -smp 1 but now we have added some > aarch64 guests we can do better when running on x86_64 hardware. > Raise the limit for TCG guests when it is safe to do so. > > Signed-off-by: Alex Bennée > --- > tests/vm/basevm.py | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py > index 93859362606..dd96a6d4af6 100644 > --- a/tests/vm/basevm.py > +++ b/tests/vm/basevm.py > @@ -553,6 +553,12 @@ def parse_args(vmcls): > def get_default_jobs(): > if kvm_available(vmcls.arch): > return multiprocessing.cpu_count() // 2 > +elif os.uname().machine == "x86_64" and \ > + vmcls.arch in ["aarch64", "x86_64", "i386"]: > +# MTTCG is available on these arches and we can allow more cores. > +# But only up to a reasonable limit. User can always override > +# these limits with --jobs. > +return min(multiprocessing.cpu_count() // 2, 8) > else: > return 1 > > -- > 2.20.1 > >
Re: [PATCH 01/19] configure: add --enable-tsan flag + fiber annotations for coroutine-ucontext
Hi,
On Wed, 17 Jun 2020 at 10:24, Stefan Hajnoczi wrote:
>
> On Fri, May 22, 2020 at 12:07:37PM -0400, Robert Foley wrote:
> > +#define UC_DEBUG 0
> > +#if UC_DEBUG && defined(CONFIG_TSAN)
> > +#define UC_TRACE(fmt, ...) fprintf(stderr, "%s:%d:%p " fmt "\n", \
> > +__func__, __LINE__, __tsan_get_current_fiber(), ##__VA_ARGS__);
> > +#else
> > +#define UC_TRACE(fmt, ...)
> > +#endif
>
> QEMU has tracing support, see docs/devel/tracing.txt. These fprintfs
> should be trace events defined in the util/trace-events file.
Thanks for the details. We removed this tracing in a later patch.
https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg02506.html
>
> > +
> > /**
> > * Per-thread coroutine bookkeeping
> > */
> > @@ -65,7 +80,20 @@ union cc_arg {
> > int i[2];
> > };
> >
> > -static void finish_switch_fiber(void *fake_stack_save)
> > +/* QEMU_ALWAYS_INLINE only does so if __OPTIMIZE__, so we cannot use it. */
> > +static inline __attribute__((always_inline))
>
> Please document why always_inline is necessary here and in other
> functions. Is it for performance or because the __tsan_*() functions
> need to be called from a the parent function?
We will look into this and add documentation here or (if it is no
longer needed),
remove the inline.
> > -static void start_switch_fiber(void **fake_stack_save,
> > - const void *bottom, size_t size)
> > +static inline __attribute__((always_inline)) void start_switch_fiber(
> > +CoroutineAction action, void **fake_stack_save,
> > +const void *bottom, size_t size, void *new_fiber)
> > {
> > #ifdef CONFIG_ASAN
> > -__sanitizer_start_switch_fiber(fake_stack_save, bottom, size);
> > +if (action == COROUTINE_TERMINATE) {
> > +__sanitizer_start_switch_fiber(
> > +action == COROUTINE_TERMINATE ? NULL : fake_stack_save,
>
> The if statement already checks action == COROUTINE_TERMINATE, why is it
> being checked again?
>
> I think the old behavior can be retained by dropping the if statement
> like this:
>
> __sanitizer_start_switch_fiber(action == COROUTINE_TERMINATE ?
> NULL : fake_stack_save,
> bottom, size);
>
> > +bottom, size);
Good point. We did change this by dropping the if in a later patch.
https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg02506.html
> > +}
> > +#endif
> > +#ifdef CONFIG_TSAN
> > +void *curr_fiber =
> > +__tsan_get_current_fiber();
> > +__tsan_acquire(curr_fiber);
> > +
> > +UC_TRACE("Current fiber: %p.", curr_fiber);
> > +*fake_stack_save = curr_fiber;
> > +UC_TRACE("Switch to fiber %p", new_fiber);
> > +__tsan_switch_to_fiber(new_fiber, 0); /* 0=synchronize */
> > #endif
> > }
>
> Please split start_switch_fiber() into two functions:
> start_switch_fiber_asan() and start_switch_fiber_tsan(). That way the
> asan- and tsan-specific arguments can be kept separate and the
> co->tsan_* fields only need to be compiled in when CONFIG_TSAN is
> defined.
>
> For example:
>
> static inline __attribute__((always_inline))
> void start_switch_fiber_tsan(void **fake_stack_save,
>CoroutineUContext *co,
> bool caller)
> {
> #ifdef CONFIG_TSAN
> void *new_fiber = caller ?
> co->tsan_caller_fiber :
> co->tsan_co_fiber;
> void *curr_fiber = __tsan_get_current_fiber();
> __tsan_acquire(curr_fiber);
>
> UC_TRACE("Current fiber: %p.", curr_fiber);
> *fake_stack_save = curr_fiber;
> UC_TRACE("Switch to fiber %p", new_fiber);
> __tsan_switch_to_fiber(new_fiber, 0); /* 0=synchronize */
> #endif
> }
>
> This does two things:
> 1. Unrelated ASAN and TSAN code is separate and each function only
>has arguments that are actually needed.
> 2. The co->tsan_caller_fiber and co->tsan_co_fiber fields are only
>access from within #ifdef CONFIG_TSAN.
Makes sense, we will make these changes and submit a cleanup patch.
Thanks & Regards,
-Rob
[PATCH v10 70/73] cpu: protect CPU state with cpu->lock instead of the BQL
From: "Emilio G. Cota"
Use the per-CPU locks to protect the CPUs' state, instead of
using the BQL. These locks are uncontended (they are mostly
acquired by the corresponding vCPU thread), so acquiring them
is cheaper than acquiring the BQL, which particularly in
MTTCG can be contended at high core counts.
In this conversion we drop qemu_cpu_cond and qemu_pause_cond,
and use cpu->cond instead.
In qom/cpu.c we can finally remove the ugliness that
results from having to hold both the BQL and the CPU lock;
now we just have to grab the CPU lock.
Removed g_assert(qemu_mutex_iothread_locked())
from qemu_tcg_rr_all_cpu_threads_idle(). There is an existing
case where we call qemu_tcg_rr_all_cpu_threads_idle() without
the BQL held.
Also changed qemu_cond_wait() to qemu_cond_timed_wait()
in qemu_tcg_rr_cpu_thread_fn() to fix issue where we might
miss the broadcast wakeup. Also changed to obtain cpu lock
in qemu_cpu_kick() prior to broadcast to avoid missing notification.
Reviewed-by: Alex Bennée
Reviewed-by: Richard Henderson
Signed-off-by: Emilio G. Cota
[RF: Adjusted assert, added timed_wait, obtain cpu lock]
Signed-off-by: Robert Foley
---
cpus.c| 307 --
hw/core/cpu.c | 29 +---
include/hw/core/cpu.h | 20 +--
3 files changed, 248 insertions(+), 108 deletions(-)
diff --git a/cpus.c b/cpus.c
index 2e9c19ae36..4a1056aab0 100644
--- a/cpus.c
+++ b/cpus.c
@@ -200,24 +200,30 @@ void cpu_mutex_destroy(CPUState *cpu)
}
}
-bool cpu_is_stopped(CPUState *cpu)
+/* Called with the CPU's lock held */
+static bool cpu_is_stopped_locked(CPUState *cpu)
{
return cpu->stopped || !runstate_is_running();
}
-static inline bool cpu_work_list_empty(CPUState *cpu)
+bool cpu_is_stopped(CPUState *cpu)
{
-bool ret;
+if (!cpu_mutex_locked(cpu)) {
+bool ret;
-cpu_mutex_lock(cpu);
-ret = QSIMPLEQ_EMPTY(>work_list);
-cpu_mutex_unlock(cpu);
-return ret;
+cpu_mutex_lock(cpu);
+ret = cpu_is_stopped_locked(cpu);
+cpu_mutex_unlock(cpu);
+return ret;
+}
+return cpu_is_stopped_locked(cpu);
}
static bool cpu_thread_is_idle(CPUState *cpu)
{
-if (cpu->stop || !cpu_work_list_empty(cpu)) {
+g_assert(cpu_mutex_locked(cpu));
+
+if (cpu->stop || !QSIMPLEQ_EMPTY(>work_list)) {
return false;
}
if (cpu_is_stopped(cpu)) {
@@ -235,9 +241,16 @@ static bool qemu_tcg_rr_all_cpu_threads_idle(void)
CPUState *cpu;
g_assert(qemu_is_tcg_rr());
+g_assert(no_cpu_mutex_locked());
CPU_FOREACH(cpu) {
-if (!cpu_thread_is_idle(cpu)) {
+bool is_idle;
+
+cpu_mutex_lock(cpu);
+is_idle = cpu_thread_is_idle(cpu);
+cpu_mutex_unlock(cpu);
+
+if (!is_idle) {
return false;
}
}
@@ -731,6 +744,8 @@ void qemu_start_warp_timer(void)
static void qemu_account_warp_timer(void)
{
+g_assert(qemu_mutex_iothread_locked());
+
if (!use_icount || !icount_sleep) {
return;
}
@@ -865,8 +880,15 @@ static void cpu_throttle_thread(CPUState *cpu,
run_on_cpu_data opaque)
endtime_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + sleeptime_ns;
while (sleeptime_ns > 0 && !cpu->stop) {
if (sleeptime_ns > SCALE_MS) {
-qemu_cond_timedwait(cpu->halt_cond, _global_mutex,
-sleeptime_ns / SCALE_MS);
+if (!cpu_mutex_locked(cpu)) {
+cpu_mutex_lock(cpu);
+qemu_cond_timedwait(cpu->halt_cond, cpu->lock,
+sleeptime_ns / SCALE_MS);
+cpu_mutex_unlock(cpu);
+} else {
+qemu_cond_timedwait(cpu->halt_cond, cpu->lock,
+sleeptime_ns / SCALE_MS);
+}
} else {
qemu_mutex_unlock_iothread();
g_usleep(sleeptime_ns / SCALE_US);
@@ -1086,6 +1108,7 @@ static void kick_tcg_thread(void *opaque)
static void start_tcg_kick_timer(void)
{
assert(!mttcg_enabled);
+g_assert(qemu_mutex_iothread_locked());
if (!tcg_kick_vcpu_timer && CPU_NEXT(first_cpu)) {
tcg_kick_vcpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
kick_tcg_thread, NULL);
@@ -1098,6 +1121,7 @@ static void start_tcg_kick_timer(void)
static void stop_tcg_kick_timer(void)
{
assert(!mttcg_enabled);
+g_assert(qemu_mutex_iothread_locked());
if (tcg_kick_vcpu_timer && timer_pending(tcg_kick_vcpu_timer)) {
timer_del(tcg_kick_vcpu_timer);
}
@@ -1199,6 +1223,8 @@ int vm_shutdown(void)
static bool cpu_can_run(CPUState *cpu)
{
+g_assert(cpu_mutex_locked(cpu));
+
if (cpu->stop) {
return false;
}
@@ -1271,16 +1297,9 @@ static void qemu_init_sigbus(void)
static QemuThread io_thread;
-/* cpu creation */
-sta
[PATCH v10 71/73] cpus-common: release BQL earlier in run_on_cpu
From: "Emilio G. Cota"
After completing the conversion to per-CPU locks, there is no need
to release the BQL after having called cpu_kick.
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
cpus-common.c | 20 +---
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 0287018dc4..eecd63029a 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -135,6 +135,11 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func,
run_on_cpu_data data)
return;
}
+/* We are going to sleep on the CPU lock, so release the BQL */
+if (has_bql) {
+qemu_mutex_unlock_iothread();
+}
+
wi.func = func;
wi.data = data;
wi.done = false;
@@ -143,21 +148,6 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func,
run_on_cpu_data data)
cpu_mutex_lock(cpu);
queue_work_on_cpu_locked(cpu, );
-
-/*
- * We are going to sleep on the CPU lock, so release the BQL.
- *
- * During the transition to per-CPU locks, we release the BQL _after_
- * having kicked the destination CPU (from queue_work_on_cpu_locked above).
- * This makes sure that the enqueued work will be seen by the CPU
- * after being woken up from the kick, since the CPU sleeps on the BQL.
- * Once we complete the transition to per-CPU locks, we will release
- * the BQL earlier in this function.
- */
-if (has_bql) {
-qemu_mutex_unlock_iothread();
-}
-
while (!atomic_mb_read()) {
CPUState *self_cpu = current_cpu;
--
2.17.1
[PATCH v10 68/73] xtensa: convert to cpu_has_work_with_iothread_lock
From: "Emilio G. Cota" Soon we will call cpu_has_work without the BQL. Cc: Max Filippov Reviewed-by: Richard Henderson Signed-off-by: Emilio G. Cota Signed-off-by: Robert Foley --- target/xtensa/cpu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c index fda4a73722..0f96483563 100644 --- a/target/xtensa/cpu.c +++ b/target/xtensa/cpu.c @@ -47,6 +47,8 @@ static bool xtensa_cpu_has_work(CPUState *cs) #ifndef CONFIG_USER_ONLY XtensaCPU *cpu = XTENSA_CPU(cs); +g_assert(qemu_mutex_iothread_locked()); + return !cpu->env.runstall && cpu->env.pending_irq_level; #else return true; @@ -187,7 +189,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data) device_class_set_parent_reset(dc, xtensa_cpu_reset, >parent_reset); cc->class_by_name = xtensa_cpu_class_by_name; -cc->has_work = xtensa_cpu_has_work; +cc->has_work_with_iothread_lock = xtensa_cpu_has_work; cc->do_interrupt = xtensa_cpu_do_interrupt; cc->cpu_exec_interrupt = xtensa_cpu_exec_interrupt; cc->dump_state = xtensa_cpu_dump_state; -- 2.17.1
[PATCH v10 65/73] s390x: convert to cpu_has_work_with_iothread_lock
From: "Emilio G. Cota"
Soon we will call cpu_has_work without the BQL.
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Reviewed-by: Richard Henderson
Reviewed-by: Cornelia Huck
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/s390x/cpu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index d157dbb7a9..4d0d323cf9 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -59,6 +59,8 @@ static bool s390_cpu_has_work(CPUState *cs)
{
S390CPU *cpu = S390_CPU(cs);
+g_assert(qemu_mutex_iothread_locked());
+
/* STOPPED cpus can never wake up */
if (s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD &&
s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING) {
@@ -489,7 +491,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
#endif
scc->reset = s390_cpu_reset;
cc->class_by_name = s390_cpu_class_by_name,
-cc->has_work = s390_cpu_has_work;
+cc->has_work_with_iothread_lock = s390_cpu_has_work;
#ifdef CONFIG_TCG
cc->do_interrupt = s390_cpu_do_interrupt;
#endif
--
2.17.1
[PATCH v10 63/73] ppc: convert to cpu_has_work_with_iothread_lock
From: "Emilio G. Cota"
Soon we will call cpu_has_work without the BQL.
Cc: qemu-...@nongnu.org
Reviewed-by: Richard Henderson
Acked-by: David Gibson
Signed-off-by: Emilio G. Cota
[RF: Converted new code related to POWER10]
Signed-off-by: Robert Foley
---
target/ppc/translate_init.inc.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index d06d9ff204..60ee51c105 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -8540,6 +8540,8 @@ static bool cpu_has_work_POWER7(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
+g_assert(qemu_mutex_iothread_locked());
+
if (cpu_halted(cs)) {
if (!(cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD)) {
return false;
@@ -8582,7 +8584,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER7;
pcc->check_pow = check_pow_nocheck;
-cc->has_work = cpu_has_work_POWER7;
+cc->has_work_with_iothread_lock = cpu_has_work_POWER7;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -8702,6 +8704,8 @@ static bool cpu_has_work_POWER8(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
+g_assert(qemu_mutex_iothread_locked());
+
if (cpu_halted(cs)) {
if (!(cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD)) {
return false;
@@ -8752,7 +8756,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER8;
pcc->check_pow = check_pow_nocheck;
-cc->has_work = cpu_has_work_POWER8;
+cc->has_work_with_iothread_lock = cpu_has_work_POWER8;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -8902,6 +8906,8 @@ static bool cpu_has_work_POWER9(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
+g_assert(qemu_mutex_iothread_locked());
+
if (cpu_halted(cs)) {
uint64_t psscr = env->spr[SPR_PSSCR];
@@ -8969,7 +8975,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER9;
pcc->check_pow = check_pow_nocheck;
-cc->has_work = cpu_has_work_POWER9;
+cc->has_work_with_iothread_lock = cpu_has_work_POWER9;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -9118,6 +9124,7 @@ static bool cpu_has_work_POWER10(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
+g_assert(qemu_mutex_iothread_locked());
if (cpu_halted(cs)) {
uint64_t psscr = env->spr[SPR_PSSCR];
@@ -9186,7 +9193,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
pcc->init_proc = init_proc_POWER10;
pcc->check_pow = check_pow_nocheck;
-cc->has_work = cpu_has_work_POWER10;
+cc->has_work_with_iothread_lock = cpu_has_work_POWER10;
pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -10656,6 +10663,8 @@ static bool ppc_cpu_has_work(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
+g_assert(qemu_mutex_iothread_locked());
+
return msr_ee && (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD);
}
@@ -10880,7 +10889,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void
*data)
cc->class_by_name = ppc_cpu_class_by_name;
pcc->parent_parse_features = cc->parse_features;
cc->parse_features = ppc_cpu_parse_featurestr;
-cc->has_work = ppc_cpu_has_work;
+cc->has_work_with_iothread_lock = ppc_cpu_has_work;
cc->do_interrupt = ppc_cpu_do_interrupt;
cc->cpu_exec_interrupt = ppc_cpu_exec_interrupt;
cc->dump_state = ppc_cpu_dump_state;
--
2.17.1
[PATCH v10 61/73] cpu: call .cpu_has_work with the CPU lock held
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
include/hw/core/cpu.h | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 6f2c005171..79da78cd10 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -80,7 +80,8 @@ struct TranslationBlock;
* instantiatable CPU type.
* @parse_features: Callback to parse command line arguments.
* @reset_dump_flags: #CPUDumpFlags to use for reset logging.
- * @has_work: Callback for checking if there is work to do.
+ * @has_work: Callback for checking if there is work to do. Called with the
+ * CPU lock held.
* @do_interrupt: Callback for interrupt handling.
* @do_unaligned_access: Callback for unaligned access handling, if
* the target defines #TARGET_ALIGNED_ONLY.
@@ -804,9 +805,16 @@ const char *parse_cpu_option(const char *cpu_option);
static inline bool cpu_has_work(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
+bool ret;
g_assert(cc->has_work);
-return cc->has_work(cpu);
+if (cpu_mutex_locked(cpu)) {
+return cc->has_work(cpu);
+}
+cpu_mutex_lock(cpu);
+ret = cc->has_work(cpu);
+cpu_mutex_unlock(cpu);
+return ret;
}
/**
--
2.17.1
[PATCH v10 59/73] accel/tcg: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
accel/tcg/cpu-exec.c | 15 ---
accel/tcg/tcg-all.c | 12 +---
accel/tcg/translate-all.c | 2 +-
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 099dd83ee0..b549a37847 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -431,7 +431,7 @@ static inline bool cpu_handle_halt_locked(CPUState *cpu)
if (cpu_halted(cpu)) {
#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
-if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
+if ((cpu_interrupt_request(cpu) & CPU_INTERRUPT_POLL)
&& replay_interrupt()) {
X86CPU *x86_cpu = X86_CPU(cpu);
@@ -544,16 +544,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
*/
atomic_mb_set(_neg(cpu)->icount_decr.u16.high, 0);
-if (unlikely(atomic_read(>interrupt_request))) {
+if (unlikely(cpu_interrupt_request(cpu))) {
int interrupt_request;
+
qemu_mutex_lock_iothread();
-interrupt_request = cpu->interrupt_request;
+interrupt_request = cpu_interrupt_request(cpu);
if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
/* Mask out external interrupts for this step. */
interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
}
if (interrupt_request & CPU_INTERRUPT_DEBUG) {
-cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
+cpu_reset_interrupt(cpu, CPU_INTERRUPT_DEBUG);
cpu->exception_index = EXCP_DEBUG;
qemu_mutex_unlock_iothread();
return true;
@@ -562,7 +563,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
/* Do nothing */
} else if (interrupt_request & CPU_INTERRUPT_HALT) {
replay_interrupt();
-cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
+cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT);
cpu_halted_set(cpu, 1);
cpu->exception_index = EXCP_HLT;
qemu_mutex_unlock_iothread();
@@ -599,10 +600,10 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
}
/* The target hook may have updated the 'cpu->interrupt_request';
* reload the 'interrupt_request' value */
-interrupt_request = cpu->interrupt_request;
+interrupt_request = cpu_interrupt_request(cpu);
}
if (interrupt_request & CPU_INTERRUPT_EXITTB) {
-cpu->interrupt_request &= ~CPU_INTERRUPT_EXITTB;
+cpu_reset_interrupt(cpu, CPU_INTERRUPT_EXITTB);
/* ensure that no TB jump will be modified as
the program flow was changed */
*last_tb = NULL;
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 3b4fda5640..5eda24d87b 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -52,10 +52,16 @@ typedef struct TCGState {
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;
+if (!cpu_mutex_locked(cpu)) {
+cpu_mutex_lock(cpu);
+old_mask = cpu_interrupt_request(cpu);
+cpu_interrupt_request_or(cpu, mask);
+cpu_mutex_unlock(cpu);
+} else {
+old_mask = cpu_interrupt_request(cpu);
+cpu_interrupt_request_or(cpu, mask);
+}
/*
* If called from iothread context, wake the target cpu in
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index c3d37058a1..23007570f6 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2409,7 +2409,7 @@ void dump_opcount_info(void)
void cpu_interrupt(CPUState *cpu, int mask)
{
g_assert(qemu_mutex_iothread_locked());
-cpu->interrupt_request |= mask;
+cpu_interrupt_request_or(cpu, mask);
atomic_set(_neg(cpu)->icount_decr.u16.high, -1);
}
--
2.17.1
[PATCH v10 60/73] cpu: convert to interrupt_request
From: "Emilio G. Cota"
This finishes the conversion to interrupt_request.
Reviewed-by: Alex Bennée
Reviewed-by: Richard Henderson
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
hw/core/cpu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index d3223f6d42..59f02594a3 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -273,7 +273,7 @@ static void cpu_common_reset(DeviceState *dev)
log_cpu_state(cpu, cc->reset_dump_flags);
}
-cpu->interrupt_request = 0;
+cpu_interrupt_request_set(cpu, 0);
cpu_halted_set(cpu, 0);
cpu->mem_io_pc = 0;
cpu->icount_extra = 0;
@@ -413,7 +413,7 @@ static vaddr cpu_adjust_watchpoint_address(CPUState *cpu,
vaddr addr, int len)
static void generic_handle_interrupt(CPUState *cpu, int mask)
{
-cpu->interrupt_request |= mask;
+cpu_interrupt_request_or(cpu, mask);
if (!qemu_cpu_is_self(cpu)) {
qemu_cpu_kick(cpu);
--
2.17.1
[PATCH v10 57/73] unicore32: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Guan Xuetao
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/unicore32/cpu.c | 2 +-
target/unicore32/softmmu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/unicore32/cpu.c b/target/unicore32/cpu.c
index b27fb9689f..06bf4b4b63 100644
--- a/target/unicore32/cpu.c
+++ b/target/unicore32/cpu.c
@@ -27,7 +27,7 @@ static void uc32_cpu_set_pc(CPUState *cs, vaddr value)
static bool uc32_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request &
+return cpu_interrupt_request(cs) &
(CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB);
}
diff --git a/target/unicore32/softmmu.c b/target/unicore32/softmmu.c
index cbdaa500b7..9660bd2a27 100644
--- a/target/unicore32/softmmu.c
+++ b/target/unicore32/softmmu.c
@@ -117,7 +117,7 @@ void uc32_cpu_do_interrupt(CPUState *cs)
/* The PC already points to the proper instruction. */
env->regs[30] = env->regs[31];
env->regs[31] = addr;
-cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
+cpu_interrupt_request_or(cs, CPU_INTERRUPT_EXITTB);
}
static int get_phys_addr_ucv2(CPUUniCore32State *env, uint32_t address,
--
2.17.1
[PATCH v10 51/73] nios: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Chris Wulff
Cc: Marek Vasut
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/nios2/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index 8f7011fcb9..fe5fd9adfd 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -36,7 +36,7 @@ static void nios2_cpu_set_pc(CPUState *cs, vaddr value)
static bool nios2_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
+return cpu_interrupt_request(cs) & (CPU_INTERRUPT_HARD |
CPU_INTERRUPT_NMI);
}
static void nios2_cpu_reset(DeviceState *dev)
--
2.17.1
[PATCH v10 69/73] cpu: rename all_cpu_threads_idle to qemu_tcg_rr_all_cpu_threads_idle
From: "Emilio G. Cota"
This function is only called from TCG rr mode, so add
a prefix to mark this as well as an assertion.
Reviewed-by: Alex Bennée
Reviewed-by: Richard Henderson
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
cpus.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/cpus.c b/cpus.c
index e070bffbff..2e9c19ae36 100644
--- a/cpus.c
+++ b/cpus.c
@@ -230,10 +230,12 @@ static bool cpu_thread_is_idle(CPUState *cpu)
return true;
}
-static bool all_cpu_threads_idle(void)
+static bool qemu_tcg_rr_all_cpu_threads_idle(void)
{
CPUState *cpu;
+g_assert(qemu_is_tcg_rr());
+
CPU_FOREACH(cpu) {
if (!cpu_thread_is_idle(cpu)) {
return false;
@@ -642,7 +644,7 @@ void qemu_start_warp_timer(void)
}
if (replay_mode != REPLAY_MODE_PLAY) {
-if (!all_cpu_threads_idle()) {
+if (!qemu_tcg_rr_all_cpu_threads_idle()) {
return;
}
@@ -1320,7 +1322,7 @@ static void qemu_tcg_rr_wait_io_event(void)
{
CPUState *cpu;
-while (all_cpu_threads_idle()) {
+while (qemu_tcg_rr_all_cpu_threads_idle()) {
stop_tcg_kick_timer();
qemu_cond_wait(first_cpu->halt_cond, _global_mutex);
}
@@ -1676,7 +1678,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
atomic_mb_set(>exit_request, 0);
}
-if (use_icount && all_cpu_threads_idle()) {
+if (use_icount && qemu_tcg_rr_all_cpu_threads_idle()) {
/*
* When all cpus are sleeping (e.g in WFI), to avoid a deadlock
* in the main_loop, wake it up in order to start the warp timer.
--
2.17.1
[PATCH v10 50/73] mips: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Aurelien Jarno
Cc: Aleksandar Markovic
Reviewed-by: Richard Henderson
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/mips/cpu.c | 7 ---
target/mips/kvm.c | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index e86cd06548..761d8aaa54 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -56,13 +56,14 @@ static bool mips_cpu_has_work(CPUState *cs)
MIPSCPU *cpu = MIPS_CPU(cs);
CPUMIPSState *env = >env;
bool has_work = false;
+uint32_t interrupt_request = cpu_interrupt_request(cs);
/*
* Prior to MIPS Release 6 it is implementation dependent if non-enabled
* interrupts wake-up the CPU, however most of the implementations only
* check for interrupts that can be taken.
*/
-if ((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+if ((interrupt_request & CPU_INTERRUPT_HARD) &&
cpu_mips_hw_interrupts_pending(env)) {
if (cpu_mips_hw_interrupts_enabled(env) ||
(env->insn_flags & ISA_MIPS32R6)) {
@@ -76,7 +77,7 @@ static bool mips_cpu_has_work(CPUState *cs)
* The QEMU model will issue an _WAKE request whenever the CPUs
* should be woken up.
*/
-if (cs->interrupt_request & CPU_INTERRUPT_WAKE) {
+if (interrupt_request & CPU_INTERRUPT_WAKE) {
has_work = true;
}
@@ -86,7 +87,7 @@ static bool mips_cpu_has_work(CPUState *cs)
}
/* MIPS Release 6 has the ability to halt the CPU. */
if (env->CP0_Config5 & (1 << CP0C5_VP)) {
-if (cs->interrupt_request & CPU_INTERRUPT_WAKE) {
+if (interrupt_request & CPU_INTERRUPT_WAKE) {
has_work = true;
}
if (!mips_vp_active(env)) {
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 92608cfe15..682b969579 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -141,7 +141,7 @@ void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
qemu_mutex_lock_iothread();
-if ((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+if ((cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD) &&
cpu_mips_io_interrupts_pending(cpu)) {
intr.cpu = -1;
intr.irq = 2;
--
2.17.1
[PATCH v10 56/73] openrisc: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Stafford Horne
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
hw/openrisc/cputimer.c | 2 +-
target/openrisc/cpu.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
index 93268815d8..9475b77d0a 100644
--- a/hw/openrisc/cputimer.c
+++ b/hw/openrisc/cputimer.c
@@ -102,7 +102,7 @@ static void openrisc_timer_cb(void *opaque)
CPUState *cs = CPU(cpu);
cpu->env.ttmr |= TTMR_IP;
-cs->interrupt_request |= CPU_INTERRUPT_TIMER;
+cpu_interrupt_request_or(cs, CPU_INTERRUPT_TIMER);
}
switch (cpu->env.ttmr & TTMR_M) {
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 5528c0918f..fd2da39124 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -32,8 +32,8 @@ static void openrisc_cpu_set_pc(CPUState *cs, vaddr value)
static bool openrisc_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & (CPU_INTERRUPT_HARD |
-CPU_INTERRUPT_TIMER);
+return cpu_interrupt_request(cs) & (CPU_INTERRUPT_HARD |
+CPU_INTERRUPT_TIMER);
}
static void openrisc_disas_set_info(CPUState *cpu, disassemble_info *info)
--
2.17.1
[PATCH v10 67/73] sparc: convert to cpu_has_work_with_iothread_lock
From: "Emilio G. Cota" Soon we will call cpu_has_work without the BQL. Cc: Artyom Tarasenko Reviewed-by: Richard Henderson Acked-by: Mark Cave-Ayland Signed-off-by: Emilio G. Cota Signed-off-by: Robert Foley --- target/sparc/cpu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c index cef25238a5..3fa1a535d9 100644 --- a/target/sparc/cpu.c +++ b/target/sparc/cpu.c @@ -704,6 +704,8 @@ static bool sparc_cpu_has_work(CPUState *cs) SPARCCPU *cpu = SPARC_CPU(cs); CPUSPARCState *env = >env; +g_assert(qemu_mutex_iothread_locked()); + return (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD) && cpu_interrupts_enabled(env); } @@ -863,7 +865,7 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data) cc->class_by_name = sparc_cpu_class_by_name; cc->parse_features = sparc_cpu_parse_features; -cc->has_work = sparc_cpu_has_work; +cc->has_work_with_iothread_lock = sparc_cpu_has_work; cc->do_interrupt = sparc_cpu_do_interrupt; cc->cpu_exec_interrupt = sparc_cpu_exec_interrupt; cc->dump_state = sparc_cpu_dump_state; -- 2.17.1
[PATCH v10 47/73] hppa: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/hppa/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 71b6aca45d..287055f96e 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -61,7 +61,7 @@ static void hppa_cpu_synchronize_from_tb(CPUState *cs,
TranslationBlock *tb)
static bool hppa_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & CPU_INTERRUPT_HARD;
+return cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
}
static void hppa_cpu_disas_set_info(CPUState *cs, disassemble_info *info)
--
2.17.1
[PATCH v10 66/73] riscv: convert to cpu_has_work_with_iothread_lock
From: "Emilio G. Cota" Soon we will call cpu_has_work without the BQL. Cc: Sagar Karandikar Cc: Bastian Koppelmann Reviewed-by: Palmer Dabbelt Reviewed-by: Richard Henderson Reviewed-by: Alistair Francis Signed-off-by: Emilio G. Cota Signed-off-by: Robert Foley --- target/riscv/cpu.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 3a6d202d03..d56ba65b32 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -310,6 +310,9 @@ static bool riscv_cpu_has_work(CPUState *cs) #ifndef CONFIG_USER_ONLY RISCVCPU *cpu = RISCV_CPU(cs); CPURISCVState *env = >env; + +g_assert(qemu_mutex_iothread_locked()); + /* * Definition of the WFI instruction requires it to ignore the privilege * mode and delegation registers, but respect individual enables @@ -510,7 +513,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) device_class_set_parent_reset(dc, riscv_cpu_reset, >parent_reset); cc->class_by_name = riscv_cpu_class_by_name; -cc->has_work = riscv_cpu_has_work; +cc->has_work_with_iothread_lock = riscv_cpu_has_work; cc->do_interrupt = riscv_cpu_do_interrupt; cc->cpu_exec_interrupt = riscv_cpu_exec_interrupt; cc->dump_state = riscv_cpu_dump_state; -- 2.17.1
[PATCH v10 54/73] moxie: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Anthony Green
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/moxie/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/moxie/cpu.c b/target/moxie/cpu.c
index 6e0443ccb7..f823eb234d 100644
--- a/target/moxie/cpu.c
+++ b/target/moxie/cpu.c
@@ -32,7 +32,7 @@ static void moxie_cpu_set_pc(CPUState *cs, vaddr value)
static bool moxie_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & CPU_INTERRUPT_HARD;
+return cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
}
static void moxie_cpu_reset(DeviceState *dev)
--
2.17.1
[PATCH v10 73/73] cputlb: queue async flush jobs without the BQL
From: "Emilio G. Cota" This yields sizable scalability improvements, as the below results show. Host: Two Intel Xeon Silver 4114 20-core CPUs at 2.20 GHz VM: Ubuntu 18.04 ppc64 Speedup vs a single thread for kernel build 7 +---+ | + + + + + + | | ### baseline *** | | # cpu lock ### | | ## | 6 |-+ ## ## +-| | ## | | ## ### | | ## * # | | ## *** # | | ## *** * | 5 |-+ ## *** +-| | # ** | | # ** ** | | #* ** | | #* ** | | #* * | | # ** | | # ** | | # * | 3 |-+ # +-| | # | | # | | # | | # | 2 |-+ # +-| | # | | # | | # | | # | | # + + + + + + | 1 +---+ 0 5 10 15 20 25 30 35 Guest vCPUs Pictures are also here: https://drive.google.com/file/d/1ASg5XyP9hNfN9VysXC3qe5s9QSJlwFAt/view?usp=sharing Some notes: - baseline corresponds to the commit before this series - cpu-lock is this series Single-threaded performance is affected very lightly. Results below for debian aarch64 bootup+test for the entire series on an Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz host: - Before: Performance counter stats for 'taskset -c 0 ../img/aarch64/die.sh' (10 runs): 7269.033478 task-clock (msec) #0.998 CPUs utilized ( +- 0.06% ) 30,659,870,302 cycles#4.218 GHz ( +- 0.06% ) 54,790,540,051 instructions #1.79 insns per cycle ( +- 0.05% ) 9,796,441,380 branches # 1347.695 M/sec ( +- 0.05% ) 165,132,201 branch-misses #1.69% of all branches ( +- 0.12% ) 7.287011656 seconds time elapsed ( +- 0.10% ) - After: 7375.924053 task-clock (msec) #0.998 CPUs utilized ( +- 0.13% ) 31,107,548,846 cycles#4.217 GHz ( +- 0.12% ) 55,355,668,947 instructions #1.78 insns per cycle ( +- 0.05% ) 9,929,917,664 branches # 1346.261 M/sec ( +- 0.04% ) 166,547,442 branch-misses #1.68% of all branches ( +- 0.09% ) 7.389068145 seconds time elapsed ( +- 0.13% ) That is, a 1.37% slowdown. Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson Tested-by: Alex Bennée Signed-off-by: Emilio G. Cota [Updated the speedup chart results for re-based series.] Signed-off-by: Robert Foley --- accel/tcg/cputlb.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index 1e815357c7..7f75054643 100644 --- a/accel/tcg/cputlb.c +++ b
[PATCH v10 49/73] m68k: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Reviewed-by: Laurent Vivier
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/m68k/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 72c545149e..f2585154f5 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -33,7 +33,7 @@ static void m68k_cpu_set_pc(CPUState *cs, vaddr value)
static bool m68k_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & CPU_INTERRUPT_HARD;
+return cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
}
static void m68k_set_feature(CPUM68KState *env, int feature)
--
2.17.1
[PATCH v10 62/73] cpu: introduce cpu_has_work_with_iothread_lock
From: "Emilio G. Cota"
It will gain some users soon.
Suggested-by: Paolo Bonzini
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
include/hw/core/cpu.h | 37 ++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 79da78cd10..720bc85a84 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -26,6 +26,7 @@
#include "exec/memattrs.h"
#include "qapi/qapi-types-run-state.h"
#include "qemu/bitmap.h"
+#include "qemu/main-loop.h"
#include "qemu/rcu_queue.h"
#include "qemu/queue.h"
#include "qemu/thread.h"
@@ -82,6 +83,8 @@ struct TranslationBlock;
* @reset_dump_flags: #CPUDumpFlags to use for reset logging.
* @has_work: Callback for checking if there is work to do. Called with the
* CPU lock held.
+ * @has_work_with_iothread_lock: Callback for checking if there is work to do.
+ * Called with both the BQL and the CPU lock held.
* @do_interrupt: Callback for interrupt handling.
* @do_unaligned_access: Callback for unaligned access handling, if
* the target defines #TARGET_ALIGNED_ONLY.
@@ -167,6 +170,7 @@ typedef struct CPUClass {
int reset_dump_flags;
bool (*has_work)(CPUState *cpu);
+bool (*has_work_with_iothread_lock)(CPUState *cpu);
void (*do_interrupt)(CPUState *cpu);
void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
MMUAccessType access_type,
@@ -805,14 +809,41 @@ const char *parse_cpu_option(const char *cpu_option);
static inline bool cpu_has_work(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
+bool has_cpu_lock = cpu_mutex_locked(cpu);
+bool (*func)(CPUState *cpu);
bool ret;
+/* some targets require us to hold the BQL when checking for work */
+if (cc->has_work_with_iothread_lock) {
+if (qemu_mutex_iothread_locked()) {
+func = cc->has_work_with_iothread_lock;
+goto call_func;
+}
+
+if (has_cpu_lock) {
+/* avoid deadlock by acquiring the locks in order */
+cpu_mutex_unlock(cpu);
+}
+qemu_mutex_lock_iothread();
+cpu_mutex_lock(cpu);
+
+ret = cc->has_work_with_iothread_lock(cpu);
+
+qemu_mutex_unlock_iothread();
+if (!has_cpu_lock) {
+cpu_mutex_unlock(cpu);
+}
+return ret;
+}
+
g_assert(cc->has_work);
-if (cpu_mutex_locked(cpu)) {
-return cc->has_work(cpu);
+func = cc->has_work;
+ call_func:
+if (has_cpu_lock) {
+return func(cpu);
}
cpu_mutex_lock(cpu);
-ret = cc->has_work(cpu);
+ret = func(cpu);
cpu_mutex_unlock(cpu);
return ret;
}
--
2.17.1
[PATCH v10 45/73] sh4: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Aurelien Jarno
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/sh4/cpu.c| 2 +-
target/sh4/helper.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 3c68021c56..18f3448183 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -44,7 +44,7 @@ static void superh_cpu_synchronize_from_tb(CPUState *cs,
TranslationBlock *tb)
static bool superh_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & CPU_INTERRUPT_HARD;
+return cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
}
static void superh_cpu_reset(DeviceState *dev)
diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 408478ce5d..1e32365c75 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -62,7 +62,7 @@ void superh_cpu_do_interrupt(CPUState *cs)
{
SuperHCPU *cpu = SUPERH_CPU(cs);
CPUSH4State *env = >env;
-int do_irq = cs->interrupt_request & CPU_INTERRUPT_HARD;
+int do_irq = cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
int do_exp, irq_vector = cs->exception_index;
/* prioritize exceptions over interrupts */
--
2.17.1
[PATCH v10 72/73] cpu: add async_run_on_cpu_no_bql
From: "Emilio G. Cota"
Some async jobs do not need the BQL.
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
cpus-common.c | 39 ++-
include/hw/core/cpu.h | 14 ++
2 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index eecd63029a..923371a997 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -99,6 +99,7 @@ struct qemu_work_item {
run_on_cpu_func func;
run_on_cpu_data data;
bool free, exclusive, done;
+bool bql;
};
/* Called with the CPU's lock held */
@@ -145,6 +146,7 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func,
run_on_cpu_data data)
wi.done = false;
wi.free = false;
wi.exclusive = false;
+wi.bql = true;
cpu_mutex_lock(cpu);
queue_work_on_cpu_locked(cpu, );
@@ -169,6 +171,21 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func,
run_on_cpu_data data)
wi->func = func;
wi->data = data;
wi->free = true;
+wi->bql = true;
+
+queue_work_on_cpu(cpu, wi);
+}
+
+void async_run_on_cpu_no_bql(CPUState *cpu, run_on_cpu_func func,
+ run_on_cpu_data data)
+{
+struct qemu_work_item *wi;
+
+wi = g_malloc0(sizeof(struct qemu_work_item));
+wi->func = func;
+wi->data = data;
+wi->free = true;
+/* wi->bql initialized to false */
queue_work_on_cpu(cpu, wi);
}
@@ -315,6 +332,7 @@ void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func
func,
wi->data = data;
wi->free = true;
wi->exclusive = true;
+/* wi->bql initialized to false */
queue_work_on_cpu(cpu, wi);
}
@@ -339,6 +357,7 @@ static void process_queued_cpu_work_locked(CPUState *cpu)
* BQL, so it goes to sleep; start_exclusive() is sleeping too, so
* neither CPU can proceed.
*/
+g_assert(!wi->bql);
if (has_bql) {
qemu_mutex_unlock_iothread();
}
@@ -349,12 +368,22 @@ static void process_queued_cpu_work_locked(CPUState *cpu)
qemu_mutex_lock_iothread();
}
} else {
-if (has_bql) {
-wi->func(cpu, wi->data);
+if (wi->bql) {
+if (has_bql) {
+wi->func(cpu, wi->data);
+} else {
+qemu_mutex_lock_iothread();
+wi->func(cpu, wi->data);
+qemu_mutex_unlock_iothread();
+}
} else {
-qemu_mutex_lock_iothread();
-wi->func(cpu, wi->data);
-qemu_mutex_unlock_iothread();
+if (has_bql) {
+qemu_mutex_unlock_iothread();
+wi->func(cpu, wi->data);
+qemu_mutex_lock_iothread();
+} else {
+wi->func(cpu, wi->data);
+}
}
}
cpu_mutex_lock(cpu);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index d5e4c339e1..e37f64b57a 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -894,9 +894,23 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func,
run_on_cpu_data data);
* @data: Data to pass to the function.
*
* Schedules the function @func for execution on the vCPU @cpu asynchronously.
+ * See also: async_run_on_cpu_no_bql()
*/
void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data
data);
+/**
+ * async_run_on_cpu_no_bql:
+ * @cpu: The vCPU to run on.
+ * @func: The function to be executed.
+ * @data: Data to pass to the function.
+ *
+ * Schedules the function @func for execution on the vCPU @cpu asynchronously.
+ * This function is run outside the BQL.
+ * See also: async_run_on_cpu()
+ */
+void async_run_on_cpu_no_bql(CPUState *cpu, run_on_cpu_func func,
+ run_on_cpu_data data);
+
/**
* async_safe_run_on_cpu:
* @cpu: The vCPU to run on.
--
2.17.1
[PATCH v10 43/73] i386/hvf: convert to cpu_request_interrupt
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/i386/hvf/hvf.c| 8 +---
target/i386/hvf/x86hvf.c | 26 +++---
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index b3bd2285fa..01ee420185 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -262,7 +262,7 @@ static void update_apic_tpr(CPUState *cpu)
static void hvf_handle_interrupt(CPUState * cpu, int mask)
{
-cpu->interrupt_request |= mask;
+cpu_interrupt_request_or(cpu, mask);
if (!qemu_cpu_is_self(cpu)) {
qemu_cpu_kick(cpu);
}
@@ -737,10 +737,12 @@ int hvf_vcpu_exec(CPUState *cpu)
ret = 0;
switch (exit_reason) {
case EXIT_REASON_HLT: {
+uint32_t interrupt_request = cpu_interrupt_request(cpu);
+
macvm_set_rip(cpu, rip + ins_len);
-if (!((cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
+if (!((interrupt_request & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK))
-&& !(cpu->interrupt_request & CPU_INTERRUPT_NMI) &&
+&& !(interrupt_request & CPU_INTERRUPT_NMI) &&
!(idtvec_info & VMCS_IDT_VEC_VALID)) {
cpu_halted_set(cpu, 1);
ret = EXCP_HLT;
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index 8e9b60d0a7..565594bc10 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -352,6 +352,7 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
uint8_t vector;
uint64_t intr_type;
+uint32_t interrupt_request;
bool have_event = true;
if (env->interrupt_injected != -1) {
vector = env->interrupt_injected;
@@ -400,7 +401,7 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
};
}
-if (cpu_state->interrupt_request & CPU_INTERRUPT_NMI) {
+if (cpu_interrupt_request(cpu_state) & CPU_INTERRUPT_NMI) {
if (!(env->hflags2 & HF2_NMI_MASK) && !(info & VMCS_INTR_VALID)) {
cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_NMI);
info = VMCS_INTR_VALID | VMCS_INTR_T_NMI | EXCP02_NMI;
@@ -411,7 +412,7 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
}
if (!(env->hflags & HF_INHIBIT_IRQ_MASK) &&
-(cpu_state->interrupt_request & CPU_INTERRUPT_HARD) &&
+(cpu_interrupt_request(cpu_state) & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK) && !(info & VMCS_INTR_VALID)) {
int line = cpu_get_pic_interrupt(>env);
cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_HARD);
@@ -420,39 +421,42 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
VMCS_INTR_VALID | VMCS_INTR_T_HWINTR);
}
}
-if (cpu_state->interrupt_request & CPU_INTERRUPT_HARD) {
+if (cpu_interrupt_request(cpu_state) & CPU_INTERRUPT_HARD) {
vmx_set_int_window_exiting(cpu_state);
}
-return (cpu_state->interrupt_request
-& (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR));
+return cpu_interrupt_request(cpu_state) & (CPU_INTERRUPT_INIT |
+ CPU_INTERRUPT_TPR);
}
int hvf_process_events(CPUState *cpu_state)
{
X86CPU *cpu = X86_CPU(cpu_state);
CPUX86State *env = >env;
+uint32_t interrupt_request;
env->eflags = rreg(cpu_state->hvf_fd, HV_X86_RFLAGS);
-if (cpu_state->interrupt_request & CPU_INTERRUPT_INIT) {
+if (cpu_interrupt_request(cpu_state) & CPU_INTERRUPT_INIT) {
hvf_cpu_synchronize_state(cpu_state);
do_cpu_init(cpu);
}
-if (cpu_state->interrupt_request & CPU_INTERRUPT_POLL) {
+if (cpu_interrupt_request(cpu_state) & 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) &&
+
+interrupt_request = cpu_interrupt_request(cpu_state);
+if (((interrupt_request & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK)) ||
-(cpu_state->interrupt_request & CPU_INTERRUPT_NMI)) {
+(interrupt_request & CPU_INTERRUPT_NMI)) {
cpu_halted_set(cpu_state, 0);
}
-if (cpu_state->interrupt_request & CPU_INTERRUPT_SIPI) {
+if (interrupt_request & CPU_INTERRUPT_SIPI) {
hvf_cpu_synchronize_state(cpu_state);
do_cpu_sipi(cpu);
}
-if (cpu_state->interrupt_request & CPU_INTERRUPT_TPR) {
+if (cpu_interrupt_request(cpu_state) & 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,
--
2.17.1
[PATCH v10 55/73] sparc: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Artyom Tarasenko
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Reviewed-by: Mark Cave-Ayland
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
hw/sparc64/sparc64.c | 4 ++--
target/sparc/cpu.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
index fe9c678459..191ebfbb93 100644
--- a/hw/sparc64/sparc64.c
+++ b/hw/sparc64/sparc64.c
@@ -57,7 +57,7 @@ void cpu_check_irqs(CPUSPARCState *env)
/* The bit corresponding to psrpil is (1<< psrpil), the next bit
is (2 << psrpil). */
if (pil < (2 << env->psrpil)) {
-if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
+if (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD) {
trace_sparc64_cpu_check_irqs_reset_irq(env->interrupt_index);
env->interrupt_index = 0;
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
@@ -88,7 +88,7 @@ void cpu_check_irqs(CPUSPARCState *env)
break;
}
}
-} else if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
+} else if (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD) {
trace_sparc64_cpu_check_irqs_disabled(pil, env->pil_in, env->softint,
env->interrupt_index);
env->interrupt_index = 0;
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 3f05aba9d6..cef25238a5 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -704,7 +704,7 @@ static bool sparc_cpu_has_work(CPUState *cs)
SPARCCPU *cpu = SPARC_CPU(cs);
CPUSPARCState *env = >env;
-return (cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+return (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD) &&
cpu_interrupts_enabled(env);
}
--
2.17.1
[PATCH v10 44/73] ppc: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: qemu-...@nongnu.org
Reviewed-by: Richard Henderson
Acked-by: David Gibson
Signed-off-by: Emilio G. Cota
[RF: Minor adjustment for new code in hvf_inject_interrupts]
Signed-off-by: Robert Foley
---
hw/ppc/ppc.c| 2 +-
target/i386/hvf/x86hvf.c| 1 -
target/ppc/kvm.c| 2 +-
target/ppc/translate_init.inc.c | 14 +++---
4 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 0e7386ff88..89f92c4a88 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -89,7 +89,7 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
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);
+env->pending_interrupts, cpu_interrupt_request(CPU(cpu)));
if (locked) {
qemu_mutex_unlock_iothread();
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index 565594bc10..53515d0f0e 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -352,7 +352,6 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
uint8_t vector;
uint64_t intr_type;
-uint32_t interrupt_request;
bool have_event = true;
if (env->interrupt_injected != -1) {
vector = env->interrupt_injected;
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index aae2ef0ad4..084e158c34 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1350,7 +1350,7 @@ static int kvmppc_handle_halt(PowerPCCPU *cpu)
CPUState *cs = CPU(cpu);
CPUPPCState *env = >env;
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD) && (msr_ee)) {
+if (!(cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD) && (msr_ee)) {
cpu_halted_set(cs, 1);
cs->exception_index = EXCP_HLT;
}
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index ac59648541..d06d9ff204 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -8541,7 +8541,7 @@ static bool cpu_has_work_POWER7(CPUState *cs)
CPUPPCState *env = >env;
if (cpu_halted(cs)) {
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
+if (!(cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD)) {
return false;
}
if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
@@ -8565,7 +8565,7 @@ static bool cpu_has_work_POWER7(CPUState *cs)
}
return false;
} else {
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return msr_ee && (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD);
}
}
@@ -8703,7 +8703,7 @@ static bool cpu_has_work_POWER8(CPUState *cs)
CPUPPCState *env = >env;
if (cpu_halted(cs)) {
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
+if (!(cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD)) {
return false;
}
if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
@@ -8735,7 +8735,7 @@ static bool cpu_has_work_POWER8(CPUState *cs)
}
return false;
} else {
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return msr_ee && (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD);
}
}
@@ -8905,7 +8905,7 @@ static bool cpu_has_work_POWER9(CPUState *cs)
if (cpu_halted(cs)) {
uint64_t psscr = env->spr[SPR_PSSCR];
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
+if (!(cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD)) {
return false;
}
@@ -8951,7 +8951,7 @@ static bool cpu_has_work_POWER9(CPUState *cs)
}
return false;
} else {
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return msr_ee && (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD);
}
}
@@ -10656,7 +10656,7 @@ static bool ppc_cpu_has_work(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return msr_ee && (cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD);
}
static void ppc_cpu_reset(DeviceState *dev)
--
2.17.1
[PATCH v10 64/73] mips: convert to cpu_has_work_with_iothread_lock
From: "Emilio G. Cota" Soon we will call cpu_has_work without the BQL. Cc: Aurelien Jarno Cc: Aleksandar Markovic Reviewed-by: Richard Henderson Signed-off-by: Emilio G. Cota Signed-off-by: Robert Foley --- target/mips/cpu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/mips/cpu.c b/target/mips/cpu.c index 761d8aaa54..ec9dde5100 100644 --- a/target/mips/cpu.c +++ b/target/mips/cpu.c @@ -58,6 +58,8 @@ static bool mips_cpu_has_work(CPUState *cs) bool has_work = false; uint32_t interrupt_request = cpu_interrupt_request(cs); +g_assert(qemu_mutex_iothread_locked()); + /* * Prior to MIPS Release 6 it is implementation dependent if non-enabled * interrupts wake-up the CPU, however most of the implementations only @@ -193,7 +195,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data) device_class_set_parent_reset(dc, mips_cpu_reset, >parent_reset); cc->class_by_name = mips_cpu_class_by_name; -cc->has_work = mips_cpu_has_work; +cc->has_work_with_iothread_lock = mips_cpu_has_work; cc->do_interrupt = mips_cpu_do_interrupt; cc->cpu_exec_interrupt = mips_cpu_exec_interrupt; cc->dump_state = mips_cpu_dump_state; -- 2.17.1
[PATCH v10 32/73] cpu: define cpu_interrupt_request helpers
From: "Emilio G. Cota"
Add a comment about how atomic_read works here. The comment refers to
a "BQL-less CPU loop", which will materialize toward the end
of this series.
Note that the modifications to cpu_reset_interrupt are there to
avoid deadlock during the CPU lock transition; once that is complete,
cpu_interrupt_request will be simple again.
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
hw/core/cpu.c | 27 +--
include/hw/core/cpu.h | 37 +
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 64a1bf3e92..d3223f6d42 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -99,14 +99,29 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
* 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();
+bool has_bql = qemu_mutex_iothread_locked();
+bool has_cpu_lock = cpu_mutex_locked(cpu);
-if (need_lock) {
-qemu_mutex_lock_iothread();
+if (has_bql) {
+if (has_cpu_lock) {
+atomic_set(>interrupt_request, cpu->interrupt_request &
~mask);
+} else {
+cpu_mutex_lock(cpu);
+atomic_set(>interrupt_request, cpu->interrupt_request &
~mask);
+cpu_mutex_unlock(cpu);
+}
+return;
+}
+
+if (has_cpu_lock) {
+cpu_mutex_unlock(cpu);
}
-cpu->interrupt_request &= ~mask;
-if (need_lock) {
-qemu_mutex_unlock_iothread();
+qemu_mutex_lock_iothread();
+cpu_mutex_lock(cpu);
+atomic_set(>interrupt_request, cpu->interrupt_request & ~mask);
+qemu_mutex_unlock_iothread();
+if (!has_cpu_lock) {
+cpu_mutex_unlock(cpu);
}
}
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 92069ebc59..6f2c005171 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -522,6 +522,43 @@ static inline void cpu_halted_set(CPUState *cpu, uint32_t
val)
cpu_mutex_unlock(cpu);
}
+/*
+ * When sending an interrupt, setters OR the appropriate bit and kick the
+ * destination vCPU. The latter can then read interrupt_request without
+ * acquiring the CPU lock, because once the kick-induced completes, they'll
read
+ * an up-to-date interrupt_request.
+ * Setters always acquire the lock, which guarantees that (1) concurrent
+ * updates from different threads won't result in data races, and (2) the
+ * BQL-less CPU loop will always see an up-to-date interrupt_request, since the
+ * loop holds the CPU lock.
+ */
+static inline uint32_t cpu_interrupt_request(CPUState *cpu)
+{
+return atomic_read(>interrupt_request);
+}
+
+static inline void cpu_interrupt_request_or(CPUState *cpu, uint32_t mask)
+{
+if (cpu_mutex_locked(cpu)) {
+atomic_set(>interrupt_request, cpu->interrupt_request | mask);
+return;
+}
+cpu_mutex_lock(cpu);
+atomic_set(>interrupt_request, cpu->interrupt_request | mask);
+cpu_mutex_unlock(cpu);
+}
+
+static inline void cpu_interrupt_request_set(CPUState *cpu, uint32_t val)
+{
+if (cpu_mutex_locked(cpu)) {
+atomic_set(>interrupt_request, val);
+return;
+}
+cpu_mutex_lock(cpu);
+atomic_set(>interrupt_request, val);
+cpu_mutex_unlock(cpu);
+}
+
static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
{
unsigned int i;
--
2.17.1
[PATCH v10 58/73] microblaze: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Reviewed-by: Edgar E. Iglesias
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/microblaze/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 51e5c85b10..ce70f7d281 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -84,7 +84,7 @@ static void mb_cpu_set_pc(CPUState *cs, vaddr value)
static bool mb_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
+return cpu_interrupt_request(cs) & (CPU_INTERRUPT_HARD |
CPU_INTERRUPT_NMI);
}
#ifndef CONFIG_USER_ONLY
--
2.17.1
[PATCH v10 52/73] s390x: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Christian Borntraeger
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Reviewed-by: Cornelia Huck
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
hw/intc/s390_flic.c | 2 +-
target/s390x/cpu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index 46ec8272c0..964cb41b4c 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -191,7 +191,7 @@ static void qemu_s390_flic_notify(uint32_t type)
CPU_FOREACH(cs) {
S390CPU *cpu = S390_CPU(cs);
-cs->interrupt_request |= CPU_INTERRUPT_HARD;
+cpu_interrupt_request_or(cs, CPU_INTERRUPT_HARD);
/* ignore CPUs that are not sleeping */
if (s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING &&
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index d7a98bd52b..d157dbb7a9 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -65,7 +65,7 @@ static bool s390_cpu_has_work(CPUState *cs)
return false;
}
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
+if (!(cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD)) {
return false;
}
--
2.17.1
[PATCH v10 41/73] i386/hax-all: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
[RF: Converted new code to cpu_interrupt_request]
Signed-off-by: Robert Foley
---
target/i386/hax-all.c | 34 +++---
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index 6a4152730f..20a75a891f 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -298,7 +298,7 @@ int hax_vm_destroy(struct hax_vm *vm)
static void hax_handle_interrupt(CPUState *cpu, int mask)
{
-cpu->interrupt_request |= mask;
+cpu_interrupt_request_or(cpu, mask);
if (!qemu_cpu_is_self(cpu)) {
qemu_cpu_kick(cpu);
@@ -432,7 +432,7 @@ static int hax_vcpu_interrupt(CPUArchState *env)
* Unlike KVM, HAX kernel check for the eflags, instead of qemu
*/
if (ht->ready_for_interrupt_injection &&
-(cpu->interrupt_request & CPU_INTERRUPT_HARD)) {
+(cpu_interrupt_request(cpu) & CPU_INTERRUPT_HARD)) {
int irq;
irq = cpu_get_pic_interrupt(env);
@@ -446,7 +446,7 @@ static int hax_vcpu_interrupt(CPUArchState *env)
* 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 ((cpu_interrupt_request(cpu) & CPU_INTERRUPT_HARD)) {
ht->request_interrupt_window = 1;
} else {
ht->request_interrupt_window = 0;
@@ -485,7 +485,7 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
return 0;
}
-if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
+if (cpu_interrupt_request(cpu) & CPU_INTERRUPT_POLL) {
cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
apic_poll_irq(x86_cpu->apic_state);
}
@@ -508,20 +508,20 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
* c) INIT signal;
* d) SIPI signal.
*/
-if (((cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
+if (((cpu_interrupt_request(cpu) & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK)) ||
-(cpu->interrupt_request & CPU_INTERRUPT_NMI)) {
+(cpu_interrupt_request(cpu) & CPU_INTERRUPT_NMI)) {
cpu_halted_set(cpu, 0);
}
-if (cpu->interrupt_request & CPU_INTERRUPT_INIT) {
+if (cpu_interrupt_request(cpu) & CPU_INTERRUPT_INIT) {
DPRINTF("\nhax_vcpu_hax_exec: handling INIT for %d\n",
cpu->cpu_index);
do_cpu_init(x86_cpu);
hax_vcpu_sync_state(env, 1);
}
-if (cpu->interrupt_request & CPU_INTERRUPT_SIPI) {
+if (cpu_interrupt_request(cpu) & CPU_INTERRUPT_SIPI) {
DPRINTF("hax_vcpu_hax_exec: handling SIPI for %d\n",
cpu->cpu_index);
hax_vcpu_sync_state(env, 0);
@@ -590,13 +590,17 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
ret = -1;
break;
case HAX_EXIT_HLT:
-if (!(cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
-!(cpu->interrupt_request & CPU_INTERRUPT_NMI)) {
-/* hlt instruction with interrupt disabled is shutdown */
-env->eflags |= IF_MASK;
-cpu_halted_set(cpu, 1);
-cpu->exception_index = EXCP_HLT;
-ret = 1;
+{
+uint32_t interrupt_request = cpu_interrupt_request(cpu);
+
+if (!(interrupt_request & CPU_INTERRUPT_HARD) &&
+!(interrupt_request & CPU_INTERRUPT_NMI)) {
+/* hlt instruction with interrupt disabled is shutdown */
+env->eflags |= IF_MASK;
+cpu_halted_set(cpu, 1);
+cpu->exception_index = EXCP_HLT;
+ret = 1;
+}
}
break;
/* these situations will continue to hax module */
--
2.17.1
[PATCH v10 42/73] i386/whpx-all: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/i386/whpx-all.c | 41 -
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index d5beb4a5e2..cb424f04a3 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -752,12 +752,14 @@ static int whpx_handle_portio(CPUState *cpu,
static int whpx_handle_halt(CPUState *cpu)
{
struct CPUX86State *env = (CPUArchState *)(cpu->env_ptr);
+uint32_t interrupt_request;
int ret = 0;
qemu_mutex_lock_iothread();
-if (!((cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
+interrupt_request = cpu_interrupt_request(cpu);
+if (!((interrupt_request & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK)) &&
-!(cpu->interrupt_request & CPU_INTERRUPT_NMI)) {
+!(interrupt_request & CPU_INTERRUPT_NMI)) {
cpu->exception_index = EXCP_HLT;
cpu_halted_set(cpu, true);
ret = 1;
@@ -775,6 +777,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
struct CPUX86State *env = (CPUArchState *)(cpu->env_ptr);
X86CPU *x86_cpu = X86_CPU(cpu);
int irq;
+uint32_t interrupt_request;
uint8_t tpr;
WHV_X64_PENDING_INTERRUPTION_REGISTER new_int;
UINT32 reg_count = 0;
@@ -786,17 +789,19 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
qemu_mutex_lock_iothread();
+interrupt_request = cpu_interrupt_request(cpu);
+
/* Inject NMI */
if (!vcpu->interruption_pending &&
-cpu->interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
-if (cpu->interrupt_request & CPU_INTERRUPT_NMI) {
+interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) {
+if (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) {
+if (interrupt_request & CPU_INTERRUPT_SMI) {
cpu_reset_interrupt(cpu, CPU_INTERRUPT_SMI);
}
}
@@ -805,12 +810,12 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
* Force the VCPU out of its inner loop to process any INIT requests or
* commit pending TPR access.
*/
-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;
}
}
@@ -819,7 +824,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
if (!vcpu->interruption_pending &&
vcpu->interruptable && (env->eflags & IF_MASK)) {
assert(!new_int.InterruptionPending);
-if (cpu->interrupt_request & CPU_INTERRUPT_HARD) {
+if (interrupt_request & CPU_INTERRUPT_HARD) {
cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
irq = cpu_get_pic_interrupt(env);
if (irq >= 0) {
@@ -849,7 +854,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
/* Update the state of the interrupt delivery notification */
if (!vcpu->window_registered &&
-cpu->interrupt_request & CPU_INTERRUPT_HARD) {
+cpu_interrupt_request(cpu) & CPU_INTERRUPT_HARD) {
reg_values[reg_count].DeliverabilityNotifications.InterruptNotification
= 1;
vcpu->window_registered = 1;
@@ -902,31 +907,33 @@ static void whpx_vcpu_process_async_events(CPUState *cpu)
struct CPUX86State *env = (CPUArchState *)(cpu->env_ptr);
X86CPU *x86_cpu = X86_CPU(cpu);
struct whpx_vcpu *vcpu = get_whpx_vcpu(cpu);
+uint32_t interrupt_request;
-if ((cpu->interrupt_request & CPU_INTERRUPT_INIT) &&
+if ((cpu_interrupt_request(cpu) & CPU_INTERRUPT_INIT) &&
!(env->hflags & HF_SMM_MASK)) {
whpx_cpu_synchronize_state(cpu);
do_cpu_init(x86_cpu);
vcpu->interruptable = true;
}
-if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
+if (cpu_interrupt_request(cpu) & CPU_INTERRUPT_POLL) {
cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
apic_poll_irq(x86_cpu->apic_state);
}
-
[PATCH v10 38/73] arm: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/arm/cpu.c | 6 +++---
target/arm/helper.c | 16 +++-
target/arm/machine.c | 2 +-
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index df5c019145..2b11e126a8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -75,7 +75,7 @@ static bool arm_cpu_has_work(CPUState *cs)
ARMCPU *cpu = ARM_CPU(cs);
return (cpu->power_state != PSCI_OFF)
-&& cs->interrupt_request &
+&& cpu_interrupt_request(cs) &
(CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
| CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ
| CPU_INTERRUPT_EXITTB);
@@ -586,7 +586,7 @@ void arm_cpu_update_virq(ARMCPU *cpu)
bool new_state = (env->cp15.hcr_el2 & HCR_VI) ||
(env->irq_line_state & CPU_INTERRUPT_VIRQ);
-if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VIRQ) != 0)) {
+if (new_state != ((cpu_interrupt_request(cs) & CPU_INTERRUPT_VIRQ) != 0)) {
if (new_state) {
cpu_interrupt(cs, CPU_INTERRUPT_VIRQ);
} else {
@@ -607,7 +607,7 @@ void arm_cpu_update_vfiq(ARMCPU *cpu)
bool new_state = (env->cp15.hcr_el2 & HCR_VF) ||
(env->irq_line_state & CPU_INTERRUPT_VFIQ);
-if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VFIQ) != 0)) {
+if (new_state != ((cpu_interrupt_request(cs) & CPU_INTERRUPT_VFIQ) != 0)) {
if (new_state) {
cpu_interrupt(cs, CPU_INTERRUPT_VFIQ);
} else {
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 972a766730..37ac1cabfe 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2079,26 +2079,27 @@ static uint64_t isr_read(CPUARMState *env, const
ARMCPRegInfo *ri)
CPUState *cs = env_cpu(env);
uint64_t hcr_el2 = arm_hcr_el2_eff(env);
uint64_t ret = 0;
+uint32_t interrupt_request = cpu_interrupt_request(cs);
bool allow_virt = (arm_current_el(env) == 1 &&
(!arm_is_secure_below_el3(env) ||
(env->cp15.scr_el3 & SCR_EEL2)));
if (allow_virt && (hcr_el2 & HCR_IMO)) {
-if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
+if (interrupt_request & CPU_INTERRUPT_VIRQ) {
ret |= CPSR_I;
}
} else {
-if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
+if (interrupt_request & CPU_INTERRUPT_HARD) {
ret |= CPSR_I;
}
}
if (allow_virt && (hcr_el2 & HCR_FMO)) {
-if (cs->interrupt_request & CPU_INTERRUPT_VFIQ) {
+if (interrupt_request & CPU_INTERRUPT_VFIQ) {
ret |= CPSR_F;
}
} else {
-if (cs->interrupt_request & CPU_INTERRUPT_FIQ) {
+if (interrupt_request & CPU_INTERRUPT_FIQ) {
ret |= CPSR_F;
}
}
@@ -9591,10 +9592,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
}
#endif
-/* Hooks may change global state so BQL should be held, also the
- * BQL needs to be held for any modification of
- * cs->interrupt_request.
- */
+/* Hooks may change global state so BQL should be held */
g_assert(qemu_mutex_iothread_locked());
arm_call_pre_el_change_hook(cpu);
@@ -9609,7 +9607,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
arm_call_el_change_hook(cpu);
if (!kvm_enabled()) {
-cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
+cpu_interrupt_request_or(cs, CPU_INTERRUPT_EXITTB);
}
}
#endif /* !CONFIG_USER_ONLY */
diff --git a/target/arm/machine.c b/target/arm/machine.c
index c5a2114f51..42f4368de3 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -713,7 +713,7 @@ static int cpu_post_load(void *opaque, int version_id)
if (env->irq_line_state == UINT32_MAX) {
CPUState *cs = CPU(cpu);
-env->irq_line_state = cs->interrupt_request &
+env->irq_line_state = cpu_interrupt_request(cs) &
(CPU_INTERRUPT_HARD | CPU_INTERRUPT_FIQ |
CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VFIQ);
}
--
2.17.1
[PATCH v10 29/73] openrisc: convert to cpu_halted
From: "Emilio G. Cota"
Cc: Stafford Horne
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/openrisc/sys_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index d9fe6c5948..2615571ce7 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -134,7 +134,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr,
target_ulong rb)
if (env->pmr & PMR_DME || env->pmr & PMR_SME) {
cpu_restore_state(cs, GETPC(), true);
env->pc += 4;
-cs->halted = 1;
+cpu_halted_set(cs, 1);
raise_exception(cpu, EXCP_HALTED);
}
break;
--
2.17.1
[PATCH v10 53/73] alpha: convert to cpu_interrupt_request
From: "Emilio G. Cota" Reviewed-by: Richard Henderson Reviewed-by: Alex Bennée Signed-off-by: Emilio G. Cota Signed-off-by: Robert Foley --- target/alpha/cpu.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c index b3fd6643e8..09677c6c44 100644 --- a/target/alpha/cpu.c +++ b/target/alpha/cpu.c @@ -42,10 +42,10 @@ static bool alpha_cpu_has_work(CPUState *cs) assume that if a CPU really wants to stay asleep, it will mask interrupts at the chipset level, which will prevent these bits from being set in the first place. */ -return cs->interrupt_request & (CPU_INTERRUPT_HARD -| CPU_INTERRUPT_TIMER -| CPU_INTERRUPT_SMP -| CPU_INTERRUPT_MCHK); +return cpu_interrupt_request(cs) & (CPU_INTERRUPT_HARD +| CPU_INTERRUPT_TIMER +| CPU_INTERRUPT_SMP +| CPU_INTERRUPT_MCHK); } static void alpha_cpu_disas_set_info(CPUState *cpu, disassemble_info *info) -- 2.17.1
[PATCH v10 17/73] arm: convert to cpu_halted
From: "Emilio G. Cota"
Cc: Andrzej Zaborowski
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
hw/arm/omap1.c| 4 ++--
hw/arm/pxa2xx_gpio.c | 2 +-
hw/arm/pxa2xx_pic.c | 2 +-
target/arm/arm-powerctl.c | 6 +++---
target/arm/cpu.c | 2 +-
target/arm/op_helper.c| 2 +-
6 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index 6ba0df6b6d..d0963a7ff7 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -1776,7 +1776,7 @@ static uint64_t omap_clkdsp_read(void *opaque, hwaddr
addr,
case 0x18: /* DSP_SYSST */
cpu = CPU(s->cpu);
return (s->clkm.clocking_scheme << 11) | s->clkm.cold_start |
-(cpu->halted << 6); /* Quite useless... */
+(cpu_halted(cpu) << 6); /* Quite useless... */
}
OMAP_BAD_REG(addr);
@@ -3797,7 +3797,7 @@ void omap_mpu_wakeup(void *opaque, int irq, int req)
struct omap_mpu_state_s *mpu = (struct omap_mpu_state_s *) opaque;
CPUState *cpu = CPU(mpu->cpu);
-if (cpu->halted) {
+if (cpu_halted(cpu)) {
cpu_interrupt(cpu, CPU_INTERRUPT_EXITTB);
}
}
diff --git a/hw/arm/pxa2xx_gpio.c b/hw/arm/pxa2xx_gpio.c
index d6d0d0b08e..266a1d45b4 100644
--- a/hw/arm/pxa2xx_gpio.c
+++ b/hw/arm/pxa2xx_gpio.c
@@ -132,7 +132,7 @@ static void pxa2xx_gpio_set(void *opaque, int line, int
level)
pxa2xx_gpio_irq_update(s);
/* Wake-up GPIOs */
-if (cpu->halted && (mask & ~s->dir[bank] & pxa2xx_gpio_wake[bank])) {
+if (cpu_halted(cpu) && (mask & ~s->dir[bank] & pxa2xx_gpio_wake[bank])) {
cpu_interrupt(cpu, CPU_INTERRUPT_EXITTB);
}
}
diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c
index 105c5e63f2..3c58546e30 100644
--- a/hw/arm/pxa2xx_pic.c
+++ b/hw/arm/pxa2xx_pic.c
@@ -59,7 +59,7 @@ static void pxa2xx_pic_update(void *opaque)
PXA2xxPICState *s = (PXA2xxPICState *) opaque;
CPUState *cpu = CPU(s->cpu);
-if (cpu->halted) {
+if (cpu_halted(cpu)) {
mask[0] = s->int_pending[0] & (s->int_enabled[0] | s->int_idle);
mask[1] = s->int_pending[1] & (s->int_enabled[1] | s->int_idle);
if (mask[0] || mask[1]) {
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index b75f813b40..f0db413519 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -64,7 +64,7 @@ static void arm_set_cpu_on_async_work(CPUState
*target_cpu_state,
/* Initialize the cpu we are turning on */
cpu_reset(target_cpu_state);
-target_cpu_state->halted = 0;
+cpu_halted_set(target_cpu_state, 0);
if (info->target_aa64) {
if ((info->target_el < 3) && arm_feature(_cpu->env,
@@ -241,7 +241,7 @@ static void arm_set_cpu_on_and_reset_async_work(CPUState
*target_cpu_state,
/* Initialize the cpu we are turning on */
cpu_reset(target_cpu_state);
-target_cpu_state->halted = 0;
+cpu_halted_set(target_cpu_state, 0);
/* Finally set the power status */
assert(qemu_mutex_iothread_locked());
@@ -297,7 +297,7 @@ static void arm_set_cpu_off_async_work(CPUState
*target_cpu_state,
assert(qemu_mutex_iothread_locked());
target_cpu->power_state = PSCI_OFF;
-target_cpu_state->halted = 1;
+cpu_halted_set(target_cpu_state, 1);
target_cpu_state->exception_index = EXCP_HLT;
}
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5b7a36b5d7..df5c019145 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -175,7 +175,7 @@ static void arm_cpu_reset(DeviceState *dev)
env->vfp.xregs[ARM_VFP_MVFR2] = cpu->isar.mvfr2;
cpu->power_state = cpu->start_powered_off ? PSCI_OFF : PSCI_ON;
-s->halted = cpu->start_powered_off;
+cpu_halted_set(s, cpu->start_powered_off);
if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index eb0de080f1..88c86a4346 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -305,7 +305,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
}
cs->exception_index = EXCP_HLT;
-cs->halted = 1;
+cpu_halted_set(cs, 1);
cpu_loop_exit(cs);
}
--
2.17.1
[PATCH v10 48/73] lm32: convert to cpu_interrupt_request
From: "Emilio G. Cota"
Cc: Michael Walle
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/lm32/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/lm32/cpu.c b/target/lm32/cpu.c
index c50ad5fa15..9e7d8ca929 100644
--- a/target/lm32/cpu.c
+++ b/target/lm32/cpu.c
@@ -96,7 +96,7 @@ static void lm32_cpu_init_cfg_reg(LM32CPU *cpu)
static bool lm32_cpu_has_work(CPUState *cs)
{
-return cs->interrupt_request & CPU_INTERRUPT_HARD;
+return cpu_interrupt_request(cs) & CPU_INTERRUPT_HARD;
}
static void lm32_cpu_reset(DeviceState *dev)
--
2.17.1
[PATCH v10 30/73] cpu-exec: convert to cpu_halted
From: "Emilio G. Cota"
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
accel/tcg/cpu-exec.c | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index d95c4848a4..e296894ea1 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -425,14 +425,21 @@ static inline TranslationBlock *tb_find(CPUState *cpu,
return tb;
}
-static inline bool cpu_handle_halt(CPUState *cpu)
+static inline bool cpu_handle_halt_locked(CPUState *cpu)
{
-if (cpu->halted) {
+g_assert(cpu_mutex_locked(cpu));
+
+if (cpu_halted(cpu)) {
#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
&& replay_interrupt()) {
X86CPU *x86_cpu = X86_CPU(cpu);
+
+/* prevent deadlock; cpu_mutex must be acquired _after_ the BQL */
+cpu_mutex_unlock(cpu);
qemu_mutex_lock_iothread();
+cpu_mutex_lock(cpu);
+
apic_poll_irq(x86_cpu->apic_state);
cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
qemu_mutex_unlock_iothread();
@@ -442,12 +449,22 @@ static inline bool cpu_handle_halt(CPUState *cpu)
return true;
}
-cpu->halted = 0;
+cpu_halted_set(cpu, 0);
}
return false;
}
+static inline bool cpu_handle_halt(CPUState *cpu)
+{
+bool ret;
+
+cpu_mutex_lock(cpu);
+ret = cpu_handle_halt_locked(cpu);
+cpu_mutex_unlock(cpu);
+return ret;
+}
+
static inline void cpu_handle_debug_exception(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
@@ -546,7 +563,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
} else if (interrupt_request & CPU_INTERRUPT_HALT) {
replay_interrupt();
cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
-cpu->halted = 1;
+cpu_halted_set(cpu, 1);
cpu->exception_index = EXCP_HLT;
qemu_mutex_unlock_iothread();
return true;
--
2.17.1
[PATCH v10 37/73] openrisc: use cpu_reset_interrupt
From: Paolo Bonzini
Cc: Stafford Horne
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Signed-off-by: Paolo Bonzini
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
target/openrisc/sys_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 2615571ce7..e54c148d4f 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -167,7 +167,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr,
target_ulong rb)
env->ttmr = (rb & ~TTMR_IP) | ip;
} else {/* Clear IP bit. */
env->ttmr = rb & ~TTMR_IP;
-cs->interrupt_request &= ~CPU_INTERRUPT_TIMER;
+cpu_reset_interrupt(cs, CPU_INTERRUPT_TIMER);
}
cpu_openrisc_timer_update(cpu);
--
2.17.1
