* [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 19:18 ` Richard Henderson
2020-08-05 18:12 ` [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt Robert Foley
` (19 subsequent siblings)
20 siblings, 1 reply; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, cota, peter.puhov, pbonzini, alex.bennee,
Richard Henderson
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 <robert.foley@linaro.org>
---
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 = &x86_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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-05 18:12 ` [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL Robert Foley
@ 2020-08-05 19:18 ` Richard Henderson
2020-08-06 9:22 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Richard Henderson @ 2020-08-05 19:18 UTC (permalink / raw)
To: Robert Foley, qemu-devel
Cc: peter.puhov, Richard Henderson, cota, alex.bennee, pbonzini
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 <robert.foley@linaro.org>
> ---
> 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.
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-05 19:18 ` Richard Henderson
@ 2020-08-06 9:22 ` Paolo Bonzini
2020-08-06 16:11 ` Robert Foley
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2020-08-06 9:22 UTC (permalink / raw)
To: Richard Henderson, Robert Foley, qemu-devel
Cc: peter.puhov, cota, alex.bennee, Richard Henderson
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 <robert.foley@linaro.org>
>> ---
>> 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,
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-06 9:22 ` Paolo Bonzini
@ 2020-08-06 16:11 ` Robert Foley
2020-08-06 18:45 ` Paolo Bonzini
2020-08-06 20:04 ` Robert Foley
0 siblings, 2 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-06 16:11 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Richard Henderson, QEMU Developers, Emilio G. Cota, Peter Puhov,
Alex Bennée, Richard Henderson
On Thu, 6 Aug 2020 at 05:22, Paolo Bonzini <pbonzini@redhat.com> 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 <robert.foley@linaro.org>
> >> ---
> >> 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
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-06 16:11 ` Robert Foley
@ 2020-08-06 18:45 ` Paolo Bonzini
2020-08-06 20:04 ` Robert Foley
1 sibling, 0 replies; 38+ messages in thread
From: Paolo Bonzini @ 2020-08-06 18:45 UTC (permalink / raw)
To: Robert Foley
Cc: Richard Henderson, QEMU Developers, Emilio G. Cota, Peter Puhov,
Alex Bennée, Richard Henderson
On 06/08/20 18:11, Robert Foley wrote:
>> 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.
No, five patches. :)
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-06 16:11 ` Robert Foley
2020-08-06 18:45 ` Paolo Bonzini
@ 2020-08-06 20:04 ` Robert Foley
2020-08-07 22:18 ` Robert Foley
1 sibling, 1 reply; 38+ messages in thread
From: Robert Foley @ 2020-08-06 20:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Richard Henderson, QEMU Developers, Emilio G. Cota, Peter Puhov,
Alex Bennée, Richard Henderson
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 <robert.foley@linaro.org> wrote:
>
> On Thu, 6 Aug 2020 at 05:22, Paolo Bonzini <pbonzini@redhat.com> 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 <robert.foley@linaro.org>
> > >> ---
> > >> 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
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-06 20:04 ` Robert Foley
@ 2020-08-07 22:18 ` Robert Foley
2020-08-08 12:00 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Robert Foley @ 2020-08-07 22:18 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Richard Henderson, QEMU Developers, Emilio G. Cota, Peter Puhov,
Alex Bennée, Richard Henderson
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 <robert.foley@linaro.org> 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 <robert.foley@linaro.org> wrote:
> >
> > On Thu, 6 Aug 2020 at 05:22, Paolo Bonzini <pbonzini@redhat.com> 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 <robert.foley@linaro.org>
> > > >> ---
> > > >> 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
> > >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-07 22:18 ` Robert Foley
@ 2020-08-08 12:00 ` Paolo Bonzini
2020-08-10 12:54 ` Robert Foley
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2020-08-08 12:00 UTC (permalink / raw)
To: Robert Foley
Cc: Richard Henderson, QEMU Developers, Emilio G. Cota, Peter Puhov,
Alex Bennée, Richard Henderson
On 08/08/20 00:18, Robert Foley wrote:
> 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.
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.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL
2020-08-08 12:00 ` Paolo Bonzini
@ 2020-08-10 12:54 ` Robert Foley
0 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-10 12:54 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Richard Henderson, QEMU Developers, Emilio G. Cota, Peter Puhov,
Alex Bennée, Richard Henderson
On Sat, 8 Aug 2020 at 08:00, Paolo Bonzini <pbonzini@redhat.com> 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
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
2020-08-05 18:12 ` [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 19:18 ` Richard Henderson
2020-08-05 18:12 ` [PATCH v1 03/21] target/arm: " Robert Foley
` (18 subsequent siblings)
20 siblings, 1 reply; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, cota, peter.puhov, pbonzini, alex.bennee,
Richard Henderson
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = "<unknown>";
@@ -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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 ` [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt Robert Foley
@ 2020-08-05 19:18 ` Richard Henderson
2020-08-05 19:57 ` Robert Foley
0 siblings, 1 reply; 38+ messages in thread
From: Richard Henderson @ 2020-08-05 19:18 UTC (permalink / raw)
To: Robert Foley, qemu-devel
Cc: peter.puhov, Richard Henderson, cota, alex.bennee, pbonzini
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 = &cpu->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?
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 19:18 ` Richard Henderson
@ 2020-08-05 19:57 ` Robert Foley
0 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 19:57 UTC (permalink / raw)
To: Richard Henderson
Cc: QEMU Developers, Emilio G. Cota, Paolo Bonzini, Peter Puhov,
Alex Bennée, Richard Henderson
On Wed, 5 Aug 2020 at 15:18, Richard Henderson
<richard.henderson@linaro.org> 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 = &cpu->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~
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v1 03/21] target/arm: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
2020-08-05 18:12 ` [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL Robert Foley
2020-08-05 18:12 ` [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 04/21] target/avr: " Robert Foley
` (17 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, robert.foley, cota, open list:ARM TCG CPUs,
peter.puhov, pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 04/21] target/avr: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (2 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 03/21] target/arm: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-06 18:36 ` Michael Rolnik
2020-08-05 18:12 ` [PATCH v1 05/21] target/cris: " Robert Foley
` (16 subsequent siblings)
20 siblings, 1 reply; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Sarah Harris, robert.foley, cota, Michael Rolnik, peter.puhov,
pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v1 04/21] target/avr: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 ` [PATCH v1 04/21] target/avr: " Robert Foley
@ 2020-08-06 18:36 ` Michael Rolnik
2020-08-06 19:36 ` Robert Foley
0 siblings, 1 reply; 38+ messages in thread
From: Michael Rolnik @ 2020-08-06 18:36 UTC (permalink / raw)
To: Robert Foley
Cc: Sarah Harris, QEMU Developers, cota, peter.puhov, Paolo Bonzini,
Alex Bennée
[-- Attachment #1: Type: text/plain, Size: 3150 bytes --]
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
On Wed, Aug 5, 2020 at 9:17 PM Robert Foley <robert.foley@linaro.org> 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 <robert.foley@linaro.org>
> ---
> 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 = &cpu->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 = &cpu->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
[-- Attachment #2: Type: text/html, Size: 4300 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 04/21] target/avr: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-06 18:36 ` Michael Rolnik
@ 2020-08-06 19:36 ` Robert Foley
0 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-06 19:36 UTC (permalink / raw)
To: Michael Rolnik
Cc: Sarah Harris, QEMU Developers, Emilio G. Cota, Peter Puhov,
Paolo Bonzini, Alex Bennée
On Thu, 6 Aug 2020 at 14:37, Michael Rolnik <mrolnik@gmail.com> 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 <robert.foley@linaro.org> 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 <robert.foley@linaro.org>
>> ---
>> 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 = &cpu->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 = &cpu->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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v1 05/21] target/cris: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (3 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 04/21] target/avr: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 06/21] target/hppa: " Robert Foley
` (15 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, peter.puhov, cota, pbonzini, Edgar E. Iglesias,
alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 06/21] target/hppa: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (4 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 05/21] target/cris: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 07/21] target/i386: " Robert Foley
` (14 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, cota, peter.puhov, pbonzini, alex.bennee,
Richard Henderson
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 07/21] target/i386: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (5 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 06/21] target/hppa: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 08/21] target/lm32: " Robert Foley
` (13 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, Eduardo Habkost, cota, peter.puhov, pbonzini,
alex.bennee, Richard Henderson
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 08/21] target/lm32: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (6 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 07/21] target/i386: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 09/21] target/m68k: " Robert Foley
` (12 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, Michael Walle, cota, peter.puhov, pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 09/21] target/m68k: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (7 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 08/21] target/lm32: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 10/21] target/microblaze: " Robert Foley
` (11 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, Laurent Vivier, cota, peter.puhov, pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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..0c3333476a 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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 10/21] target/microblaze: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (8 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 09/21] target/m68k: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 11/21] target/mips: " Robert Foley
` (10 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, peter.puhov, cota, pbonzini, Edgar E. Iglesias,
alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 11/21] target/mips: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (9 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 10/21] target/microblaze: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 12/21] target/nios2: " Robert Foley
` (9 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Aleksandar Rikalo, robert.foley, Aleksandar Markovic, cota,
peter.puhov, pbonzini, alex.bennee, Aurelien Jarno
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 12/21] target/nios2: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (10 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 11/21] target/mips: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 13/21] target/openrisc: " Robert Foley
` (8 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Marek Vasut, robert.foley, Chris Wulff, cota, peter.puhov,
pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 13/21] target/openrisc: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (11 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 12/21] target/nios2: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 14/21] target/ppc: " Robert Foley
` (7 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, cota, peter.puhov, pbonzini, Stafford Horne, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 14/21] target/ppc: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (12 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 13/21] target/openrisc: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` Robert Foley
` (6 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, cota, open list:PowerPC TCG CPUs, peter.puhov,
pbonzini, alex.bennee, David Gibson
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 15/21] target/riscv: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 02/21] target/alpha: add BQL to do_interrupt and cpu_exec_interrupt Robert Foley
` (19 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, open list:RISC-V TCG CPUs, robert.foley,
Sagar Karandikar, Bastian Koppelmann, cota, Palmer Dabbelt,
peter.puhov, pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 15/21] target/riscv: add BQL to do_interrupt and cpu_exec_interrupt
@ 2020-08-05 18:12 ` Robert Foley
0 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, cota, pbonzini, peter.puhov, robert.foley,
Palmer Dabbelt, Alistair Francis, Sagar Karandikar,
Bastian Koppelmann, open list:RISC-V TCG CPUs
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 <robert.foley@linaro.org>
---
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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 16/21] target/rx: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (14 preceding siblings ...)
2020-08-05 18:12 ` Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-05 18:12 ` [PATCH v1 17/21] target/s390x: " Robert Foley
` (4 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, Yoshinori Sato, cota, peter.puhov, pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 17/21] target/s390x: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (15 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 16/21] target/rx: " Robert Foley
@ 2020-08-05 18:12 ` Robert Foley
2020-08-06 8:59 ` Cornelia Huck
2020-08-05 18:13 ` [PATCH v1 18/21] target/sh4: " Robert Foley
` (3 subsequent siblings)
20 siblings, 1 reply; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, robert.foley, David Hildenbrand, Cornelia Huck,
open list:S390 TCG CPUs, cota, peter.puhov, pbonzini,
alex.bennee, Richard Henderson
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v1 17/21] target/s390x: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 ` [PATCH v1 17/21] target/s390x: " Robert Foley
@ 2020-08-06 8:59 ` Cornelia Huck
2020-08-06 9:12 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Cornelia Huck @ 2020-08-06 8:59 UTC (permalink / raw)
To: Robert Foley
Cc: Thomas Huth, David Hildenbrand, qemu-devel,
open list:S390 TCG CPUs, cota, peter.puhov, pbonzini,
alex.bennee, Richard Henderson
On Wed, 5 Aug 2020 14:12:59 -0400
Robert Foley <robert.foley@linaro.org> 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 <robert.foley@linaro.org>
> ---
> 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 = &cpu->env;
> bool stopped = false;
> -
> + bool bql = !qemu_mutex_iothread_locked();
> + if (bql) {
> + qemu_mutex_lock_iothread();
> + }
I'm not sure I like that conditional locking. Can we instead create
__s390_cpu_do_interrupt() or so, move the meat of this function there,
take the bql unconditionally here, and...
> 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 = &cpu->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);
...call __s390_cpu_do_interrupt() here?
> + 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;
> }
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 17/21] target/s390x: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-06 8:59 ` Cornelia Huck
@ 2020-08-06 9:12 ` Paolo Bonzini
2020-08-06 10:03 ` Alex Bennée
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2020-08-06 9:12 UTC (permalink / raw)
To: Cornelia Huck, Robert Foley
Cc: Thomas Huth, David Hildenbrand, qemu-devel,
open list:S390 TCG CPUs, cota, peter.puhov, alex.bennee,
Richard Henderson
On 06/08/20 10:59, Cornelia Huck wrote:
>> bool stopped = false;
>> -
>> + bool bql = !qemu_mutex_iothread_locked();
>> + if (bql) {
>> + qemu_mutex_lock_iothread();
>> + }
> I'm not sure I like that conditional locking. Can we instead create
> __s390_cpu_do_interrupt() or so, move the meat of this function there,
> take the bql unconditionally here, and...
>
Agreed, except the usual convention would be s390_cpu_do_interrupt_locked.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 17/21] target/s390x: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-06 9:12 ` Paolo Bonzini
@ 2020-08-06 10:03 ` Alex Bennée
0 siblings, 0 replies; 38+ messages in thread
From: Alex Bennée @ 2020-08-06 10:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Thomas Huth, Robert Foley, David Hildenbrand, Cornelia Huck,
qemu-devel, open list:S390 TCG CPUs, cota, peter.puhov,
Richard Henderson
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 06/08/20 10:59, Cornelia Huck wrote:
>>> bool stopped = false;
>>> -
>>> + bool bql = !qemu_mutex_iothread_locked();
>>> + if (bql) {
>>> + qemu_mutex_lock_iothread();
>>> + }
>> I'm not sure I like that conditional locking. Can we instead create
>> __s390_cpu_do_interrupt() or so, move the meat of this function there,
>> take the bql unconditionally here, and...
>>
>
> Agreed, except the usual convention would be
> s390_cpu_do_interrupt_locked.
We should probably document this convention in CODING_STYLE.rst/Naming
>
> Paolo
--
Alex Bennée
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v1 18/21] target/sh4: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (16 preceding siblings ...)
2020-08-05 18:12 ` [PATCH v1 17/21] target/s390x: " Robert Foley
@ 2020-08-05 18:13 ` Robert Foley
2020-08-05 18:13 ` [PATCH v1 19/21] target/sparc: " Robert Foley
` (2 subsequent siblings)
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, Yoshinori Sato, cota, peter.puhov, pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 19/21] target/sparc: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (17 preceding siblings ...)
2020-08-05 18:13 ` [PATCH v1 18/21] target/sh4: " Robert Foley
@ 2020-08-05 18:13 ` Robert Foley
2020-08-05 18:13 ` [PATCH v1 20/21] target/unicore32: " Robert Foley
2020-08-05 18:13 ` [PATCH v1 21/21] target/xtensa: " Robert Foley
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, Mark Cave-Ayland, cota, peter.puhov, pbonzini,
alex.bennee, Artyom Tarasenko
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 20/21] target/unicore32: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (18 preceding siblings ...)
2020-08-05 18:13 ` [PATCH v1 19/21] target/sparc: " Robert Foley
@ 2020-08-05 18:13 ` Robert Foley
2020-08-05 18:13 ` [PATCH v1 21/21] target/xtensa: " Robert Foley
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, cota, peter.puhov, pbonzini, Guan Xuetao, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v1 21/21] target/xtensa: add BQL to do_interrupt and cpu_exec_interrupt
2020-08-05 18:12 [PATCH v1 00/21] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path Robert Foley
` (19 preceding siblings ...)
2020-08-05 18:13 ` [PATCH v1 20/21] target/unicore32: " Robert Foley
@ 2020-08-05 18:13 ` Robert Foley
20 siblings, 0 replies; 38+ messages in thread
From: Robert Foley @ 2020-08-05 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: robert.foley, Max Filippov, cota, peter.puhov, pbonzini, alex.bennee
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 <robert.foley@linaro.org>
---
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 = &cpu->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
^ permalink raw reply related [flat|nested] 38+ messages in thread