* [Qemu-devel] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt()
@ 2017-06-13 10:55 Thomas Huth
2017-06-13 14:14 ` Alex Bennée
2017-06-13 15:29 ` [Qemu-devel] " David Gibson
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Huth @ 2017-06-13 10:55 UTC (permalink / raw)
To: David Gibson, qemu-devel; +Cc: Alexander Graf, qemu-ppc, Alex Bennée
Since the introduction of MTTCG, using the msgsnd instruction
abort()s if being called without holding the BQL. So let's protect
that part of the code now with qemu_mutex_lock_iothread().
Buglink: https://bugs.launchpad.net/qemu/+bug/1694998
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
target/ppc/excp_helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 9cb2123..3a9f086 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -17,6 +17,7 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/helper-proto.h"
#include "exec/exec-all.h"
@@ -1132,6 +1133,7 @@ void helper_msgsnd(target_ulong rb)
return;
}
+ qemu_mutex_lock_iothread();
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *cenv = &cpu->env;
@@ -1141,5 +1143,6 @@ void helper_msgsnd(target_ulong rb)
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
}
}
+ qemu_mutex_unlock_iothread();
}
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt()
2017-06-13 10:55 [Qemu-devel] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt() Thomas Huth
@ 2017-06-13 14:14 ` Alex Bennée
2017-06-15 2:32 ` [Qemu-devel] [Qemu-ppc] " Nikunj A Dadhania
2017-06-13 15:29 ` [Qemu-devel] " David Gibson
1 sibling, 1 reply; 5+ messages in thread
From: Alex Bennée @ 2017-06-13 14:14 UTC (permalink / raw)
To: Thomas Huth; +Cc: David Gibson, qemu-devel, Alexander Graf, qemu-ppc
Thomas Huth <thuth@redhat.com> writes:
> Since the introduction of MTTCG, using the msgsnd instruction
> abort()s if being called without holding the BQL. So let's protect
> that part of the code now with qemu_mutex_lock_iothread().
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1694998
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
p.s. I was checking the ppc code for other CPU_FOREACH patterns and I
noticed the tlb_flush calls could probably use the tlb_flush_all_cpus
API instead of manually looping themselves. You should also double check
the semantics to make sure none of them need to use the _synced variant
and a cpu_exit if the flush needs to complete w.r.t the originating CPU.
> ---
> target/ppc/excp_helper.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 9cb2123..3a9f086 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -17,6 +17,7 @@
> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> */
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> #include "cpu.h"
> #include "exec/helper-proto.h"
> #include "exec/exec-all.h"
> @@ -1132,6 +1133,7 @@ void helper_msgsnd(target_ulong rb)
> return;
> }
>
> + qemu_mutex_lock_iothread();
> CPU_FOREACH(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *cenv = &cpu->env;
> @@ -1141,5 +1143,6 @@ void helper_msgsnd(target_ulong rb)
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> }
> + qemu_mutex_unlock_iothread();
> }
> #endif
--
Alex Bennée
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt()
2017-06-13 10:55 [Qemu-devel] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt() Thomas Huth
2017-06-13 14:14 ` Alex Bennée
@ 2017-06-13 15:29 ` David Gibson
1 sibling, 0 replies; 5+ messages in thread
From: David Gibson @ 2017-06-13 15:29 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, Alexander Graf, qemu-ppc, Alex Bennée
[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]
On Tue, Jun 13, 2017 at 12:55:29PM +0200, Thomas Huth wrote:
> Since the introduction of MTTCG, using the msgsnd instruction
> abort()s if being called without holding the BQL. So let's protect
> that part of the code now with qemu_mutex_lock_iothread().
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1694998
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Applied to ppc-for-2.10.
> ---
> target/ppc/excp_helper.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 9cb2123..3a9f086 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -17,6 +17,7 @@
> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> */
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> #include "cpu.h"
> #include "exec/helper-proto.h"
> #include "exec/exec-all.h"
> @@ -1132,6 +1133,7 @@ void helper_msgsnd(target_ulong rb)
> return;
> }
>
> + qemu_mutex_lock_iothread();
> CPU_FOREACH(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *cenv = &cpu->env;
> @@ -1141,5 +1143,6 @@ void helper_msgsnd(target_ulong rb)
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> }
> + qemu_mutex_unlock_iothread();
> }
> #endif
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt()
2017-06-13 14:14 ` Alex Bennée
@ 2017-06-15 2:32 ` Nikunj A Dadhania
2017-06-15 9:09 ` Alex Bennée
0 siblings, 1 reply; 5+ messages in thread
From: Nikunj A Dadhania @ 2017-06-15 2:32 UTC (permalink / raw)
To: Alex Bennée, Thomas Huth
Cc: qemu-ppc, qemu-devel, David Gibson, Benjamin Herrenschmidt
Alex Bennée <alex.bennee@linaro.org> writes:
> Thomas Huth <thuth@redhat.com> writes:
>
>> Since the introduction of MTTCG, using the msgsnd instruction
>> abort()s if being called without holding the BQL. So let's protect
>> that part of the code now with qemu_mutex_lock_iothread().
>>
>> Buglink: https://bugs.launchpad.net/qemu/+bug/1694998
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> p.s. I was checking the ppc code for other CPU_FOREACH patterns and I
> noticed the tlb_flush calls could probably use the tlb_flush_all_cpus
> API instead of manually looping themselves.
Will that be synchronous call? In PPC, we do lazy tlb flush, the tlb
flushes are batched until a synchronization point (for optimization).
The batching is achieved using a tlb_need_flush (global/local) and when
there is isync/ptesync or an exception, the actual flush is done. At
this point we need to make sure that the flush is synchronous.
> You should also double check the semantics to make sure none of them
> need to use the _synced variant and a cpu_exit if the flush needs to
> complete w.r.t the originating CPU.
Regards,
Nikunj
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt()
2017-06-15 2:32 ` [Qemu-devel] [Qemu-ppc] " Nikunj A Dadhania
@ 2017-06-15 9:09 ` Alex Bennée
0 siblings, 0 replies; 5+ messages in thread
From: Alex Bennée @ 2017-06-15 9:09 UTC (permalink / raw)
To: Nikunj A Dadhania
Cc: Thomas Huth, qemu-ppc, qemu-devel, David Gibson, Benjamin Herrenschmidt
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:
> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> Since the introduction of MTTCG, using the msgsnd instruction
>>> abort()s if being called without holding the BQL. So let's protect
>>> that part of the code now with qemu_mutex_lock_iothread().
>>>
>>> Buglink: https://bugs.launchpad.net/qemu/+bug/1694998
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> p.s. I was checking the ppc code for other CPU_FOREACH patterns and I
>> noticed the tlb_flush calls could probably use the tlb_flush_all_cpus
>> API instead of manually looping themselves.
>
> Will that be synchronous call? In PPC, we do lazy tlb flush, the tlb
> flushes are batched until a synchronization point (for optimization).
No by default the non-synced flushes will occur at the end of the
current executing block (cpu->exit_request is set and the work is done
when we exit the run-loop).
> The batching is achieved using a tlb_need_flush (global/local) and when
> there is isync/ptesync or an exception, the actual flush is done. At
> this point we need to make sure that the flush is synchronous.
If you want to ensure the flush is synchronous you need to call the
_all_cpus_synced variants and do a cpu_loop_exit in your helper. This
ensures that all the flushes queued up will be executed before execution
starts at the next PC of the calling thread.
>
>> You should also double check the semantics to make sure none of them
>> need to use the _synced variant and a cpu_exit if the flush needs to
>> complete w.r.t the originating CPU.
>
> Regards,
> Nikunj
--
Alex Bennée
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-15 9:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-13 10:55 [Qemu-devel] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt() Thomas Huth
2017-06-13 14:14 ` Alex Bennée
2017-06-15 2:32 ` [Qemu-devel] [Qemu-ppc] " Nikunj A Dadhania
2017-06-15 9:09 ` Alex Bennée
2017-06-13 15:29 ` [Qemu-devel] " David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.