* [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
@ 2022-11-18 9:18 Richard Henderson
2022-11-18 9:18 ` [PATCH for-8.0 1/7] qemu/main-loop: " Richard Henderson
` (7 more replies)
0 siblings, 8 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel
Simplify the usage of qemu_mutex_lock_iothread.
Split out for ease of review.
Doesn't actually depend on anything, but patchew only handles a single
dependency, so I need to thread the dependency through the patch sets.
Based-on: 20221111074101.2069454-1-richard.henderson@linaro.org
("tcg: Support for Int128 with helpers")
r~
Richard Henderson (7):
qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request
target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt
target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb
target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip
hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq
accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex
include/qemu/main-loop.h | 29 +++++++++++++++++++++++++++++
accel/tcg/cputlb.c | 25 ++++++++-----------------
hw/mips/mips_int.c | 11 +----------
hw/ppc/ppc.c | 10 +---------
target/ppc/excp_helper.c | 11 +----------
target/ppc/helper_regs.c | 14 ++++----------
target/riscv/cpu_helper.c | 10 +---------
7 files changed, 45 insertions(+), 65 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
@ 2022-11-18 9:18 ` Richard Henderson
2022-11-18 13:30 ` Alex Bennée
2022-11-18 13:38 ` Alex Bennée
2022-11-18 9:18 ` [PATCH for-8.0 2/7] hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request Richard Henderson
` (6 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Create a wrapper for locking/unlocking the iothread lock.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:Main loop)
---
include/qemu/main-loop.h | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 3c9a9a982d..c25f390696 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -343,6 +343,35 @@ void qemu_mutex_lock_iothread_impl(const char *file, int line);
*/
void qemu_mutex_unlock_iothread(void);
+/**
+ * QEMU_IOTHREAD_LOCK_GUARD
+ *
+ * Wrap a block of code in a conditional qemu_mutex_{lock,unlock}_iothread.
+ */
+typedef struct IOThreadLockAuto IOThreadLockAuto;
+
+static inline IOThreadLockAuto *qemu_iothread_auto_lock(const char *file,
+ int line)
+{
+ if (qemu_mutex_iothread_locked()) {
+ return NULL;
+ }
+ qemu_mutex_lock_iothread_impl(file, line);
+ /* Anything non-NULL causes the cleanup function to be called */
+ return (IOThreadLockAuto *)(uintptr_t)1;
+}
+
+static inline void qemu_iothread_auto_unlock(IOThreadLockAuto *l)
+{
+ qemu_mutex_unlock_iothread();
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(IOThreadLockAuto, qemu_iothread_auto_unlock)
+
+#define QEMU_IOTHREAD_LOCK_GUARD() \
+ g_autoptr(IOThreadLockAuto) _iothread_lock_auto __attribute__((unused)) \
+ = qemu_iothread_auto_lock(__FILE__, __LINE__)
+
/*
* qemu_cond_wait_iothread: Wait on condition for the main loop mutex
*
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH for-8.0 2/7] hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
2022-11-18 9:18 ` [PATCH for-8.0 1/7] qemu/main-loop: " Richard Henderson
@ 2022-11-18 9:18 ` Richard Henderson
2022-11-21 10:55 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt Richard Henderson
` (5 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Jiaxun Yang
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
hw/mips/mips_int.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
index 2db5e10fe0..73437cd90f 100644
--- a/hw/mips/mips_int.c
+++ b/hw/mips/mips_int.c
@@ -32,17 +32,12 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
MIPSCPU *cpu = opaque;
CPUMIPSState *env = &cpu->env;
CPUState *cs = CPU(cpu);
- bool locked = false;
if (irq < 0 || irq > 7) {
return;
}
- /* Make sure locking works even if BQL is already held by the caller */
- if (!qemu_mutex_iothread_locked()) {
- locked = true;
- qemu_mutex_lock_iothread();
- }
+ QEMU_IOTHREAD_LOCK_GUARD();
if (level) {
env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
@@ -59,10 +54,6 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
} else {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
-
- if (locked) {
- qemu_mutex_unlock_iothread();
- }
}
void cpu_mips_irq_init_cpu(MIPSCPU *cpu)
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
2022-11-18 9:18 ` [PATCH for-8.0 1/7] qemu/main-loop: " Richard Henderson
2022-11-18 9:18 ` [PATCH for-8.0 2/7] hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request Richard Henderson
@ 2022-11-18 9:18 ` Richard Henderson
2022-11-18 10:12 ` Daniel Henrique Barboza
2022-11-21 10:56 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb Richard Henderson
` (4 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: qemu-ppc@nongnu.org
---
target/ppc/excp_helper.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 94adcb766b..8591bb3f73 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2163,22 +2163,13 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
void ppc_maybe_interrupt(CPUPPCState *env)
{
CPUState *cs = env_cpu(env);
- bool locked = false;
-
- if (!qemu_mutex_iothread_locked()) {
- locked = true;
- qemu_mutex_lock_iothread();
- }
+ QEMU_IOTHREAD_LOCK_GUARD();
if (ppc_next_unmasked_interrupt(env)) {
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
} else {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
-
- if (locked) {
- qemu_mutex_unlock_iothread();
- }
}
#if defined(TARGET_PPC64)
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
` (2 preceding siblings ...)
2022-11-18 9:18 ` [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt Richard Henderson
@ 2022-11-18 9:18 ` Richard Henderson
2022-11-18 10:13 ` Daniel Henrique Barboza
2022-11-21 10:57 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip Richard Henderson
` (3 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc
In addition, use tcg_enabled instead of !kvm_enabled.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: qemu-ppc@nongnu.org
---
target/ppc/helper_regs.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index c0aee5855b..779e7db513 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -22,6 +22,7 @@
#include "qemu/main-loop.h"
#include "exec/exec-all.h"
#include "sysemu/kvm.h"
+#include "sysemu/tcg.h"
#include "helper_regs.h"
#include "power8-pmu.h"
#include "cpu-models.h"
@@ -203,17 +204,10 @@ void cpu_interrupt_exittb(CPUState *cs)
{
/*
* We don't need to worry about translation blocks
- * when running with KVM.
+ * unless running with TCG.
*/
- if (kvm_enabled()) {
- return;
- }
-
- if (!qemu_mutex_iothread_locked()) {
- qemu_mutex_lock_iothread();
- cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
- qemu_mutex_unlock_iothread();
- } else {
+ if (tcg_enabled()) {
+ QEMU_IOTHREAD_LOCK_GUARD();
cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
` (3 preceding siblings ...)
2022-11-18 9:18 ` [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb Richard Henderson
@ 2022-11-18 9:18 ` Richard Henderson
2022-11-20 23:18 ` Alistair Francis
2022-11-21 10:56 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq Richard Henderson
` (2 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Alistair Francis, Bin Meng, qemu-riscv
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: Alistair Francis <alistair.francis@wdc.com>
Cc: Bin Meng <bin.meng@windriver.com>
Cc: qemu-riscv@nongnu.org
---
target/riscv/cpu_helper.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 278d163803..241d06bab8 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -610,7 +610,6 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value)
CPURISCVState *env = &cpu->env;
CPUState *cs = CPU(cpu);
uint64_t gein, vsgein = 0, vstip = 0, old = env->mip;
- bool locked = false;
if (riscv_cpu_virt_enabled(env)) {
gein = get_field(env->hstatus, HSTATUS_VGEIN);
@@ -621,10 +620,7 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value)
mask = ((mask == MIP_VSTIP) && env->vstime_irq) ? 0 : mask;
vstip = env->vstime_irq ? MIP_VSTIP : 0;
- if (!qemu_mutex_iothread_locked()) {
- locked = true;
- qemu_mutex_lock_iothread();
- }
+ QEMU_IOTHREAD_LOCK_GUARD();
env->mip = (env->mip & ~mask) | (value & mask);
@@ -634,10 +630,6 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value)
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
- if (locked) {
- qemu_mutex_unlock_iothread();
- }
-
return old;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
` (4 preceding siblings ...)
2022-11-18 9:18 ` [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip Richard Henderson
@ 2022-11-18 9:18 ` Richard Henderson
2022-11-18 10:14 ` Daniel Henrique Barboza
2022-11-21 10:57 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex Richard Henderson
2022-11-18 9:36 ` [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Paolo Bonzini
7 siblings, 2 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Cc: qemu-ppc@nongnu.org
---
hw/ppc/ppc.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index dc86c1c7db..4e816c68c7 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -44,13 +44,9 @@ void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
{
CPUPPCState *env = &cpu->env;
unsigned int old_pending;
- bool locked = false;
/* We may already have the BQL if coming from the reset path */
- if (!qemu_mutex_iothread_locked()) {
- locked = true;
- qemu_mutex_lock_iothread();
- }
+ QEMU_IOTHREAD_LOCK_GUARD();
old_pending = env->pending_interrupts;
@@ -67,10 +63,6 @@ void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
trace_ppc_irq_set_exit(env, irq, level, env->pending_interrupts,
CPU(cpu)->interrupt_request);
-
- if (locked) {
- qemu_mutex_unlock_iothread();
- }
}
/* PowerPC 6xx / 7xx internal IRQ controller */
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
` (5 preceding siblings ...)
2022-11-18 9:18 ` [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq Richard Henderson
@ 2022-11-18 9:18 ` Richard Henderson
2022-11-21 11:02 ` Philippe Mathieu-Daudé
2022-11-18 9:36 ` [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Paolo Bonzini
7 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 9:18 UTC (permalink / raw)
To: qemu-devel
Narrow the scope of the lock to the actual read/write,
moving the cpu_transation_failed call outside the lock.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cputlb.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 517b599c5f..d177afcad6 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1356,7 +1356,6 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full,
MemoryRegionSection *section;
MemoryRegion *mr;
uint64_t val;
- bool locked = false;
MemTxResult r;
section = iotlb_to_section(cpu, full->xlat_section, full->attrs);
@@ -1367,11 +1366,11 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full,
cpu_io_recompile(cpu, retaddr);
}
- if (!qemu_mutex_iothread_locked()) {
- qemu_mutex_lock_iothread();
- locked = true;
+ {
+ QEMU_IOTHREAD_LOCK_GUARD();
+ r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
}
- r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
+
if (r != MEMTX_OK) {
hwaddr physaddr = mr_offset +
section->offset_within_address_space -
@@ -1380,10 +1379,6 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full,
cpu_transaction_failed(cpu, physaddr, addr, memop_size(op), access_type,
mmu_idx, full->attrs, r, retaddr);
}
- if (locked) {
- qemu_mutex_unlock_iothread();
- }
-
return val;
}
@@ -1410,7 +1405,6 @@ static void io_writex(CPUArchState *env, CPUTLBEntryFull *full,
hwaddr mr_offset;
MemoryRegionSection *section;
MemoryRegion *mr;
- bool locked = false;
MemTxResult r;
section = iotlb_to_section(cpu, full->xlat_section, full->attrs);
@@ -1427,11 +1421,11 @@ static void io_writex(CPUArchState *env, CPUTLBEntryFull *full,
*/
save_iotlb_data(cpu, section, mr_offset);
- if (!qemu_mutex_iothread_locked()) {
- qemu_mutex_lock_iothread();
- locked = true;
+ {
+ QEMU_IOTHREAD_LOCK_GUARD();
+ r = memory_region_dispatch_write(mr, mr_offset, val, op, full->attrs);
}
- r = memory_region_dispatch_write(mr, mr_offset, val, op, full->attrs);
+
if (r != MEMTX_OK) {
hwaddr physaddr = mr_offset +
section->offset_within_address_space -
@@ -1441,9 +1435,6 @@ static void io_writex(CPUArchState *env, CPUTLBEntryFull *full,
MMU_DATA_STORE, mmu_idx, full->attrs, r,
retaddr);
}
- if (locked) {
- qemu_mutex_unlock_iothread();
- }
}
static inline target_ulong tlb_read_ofs(CPUTLBEntry *entry, size_t ofs)
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
` (6 preceding siblings ...)
2022-11-18 9:18 ` [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex Richard Henderson
@ 2022-11-18 9:36 ` Paolo Bonzini
7 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2022-11-18 9:36 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 11/18/22 10:18, Richard Henderson wrote:
> Simplify the usage of qemu_mutex_lock_iothread.
> Split out for ease of review.
>
> Doesn't actually depend on anything, but patchew only handles a single
> dependency, so I need to thread the dependency through the patch sets.
>
> Based-on: 20221111074101.2069454-1-richard.henderson@linaro.org
> ("tcg: Support for Int128 with helpers")
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> r~
>
>
> Richard Henderson (7):
> qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
> hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request
> target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt
> target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb
> target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip
> hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq
> accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex
>
> include/qemu/main-loop.h | 29 +++++++++++++++++++++++++++++
> accel/tcg/cputlb.c | 25 ++++++++-----------------
> hw/mips/mips_int.c | 11 +----------
> hw/ppc/ppc.c | 10 +---------
> target/ppc/excp_helper.c | 11 +----------
> target/ppc/helper_regs.c | 14 ++++----------
> target/riscv/cpu_helper.c | 10 +---------
> 7 files changed, 45 insertions(+), 65 deletions(-)
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt
2022-11-18 9:18 ` [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt Richard Henderson
@ 2022-11-18 10:12 ` Daniel Henrique Barboza
2022-11-21 10:56 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Daniel Henrique Barboza @ 2022-11-18 10:12 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc
On 11/18/22 06:18, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: qemu-ppc@nongnu.org
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> target/ppc/excp_helper.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 94adcb766b..8591bb3f73 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2163,22 +2163,13 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> void ppc_maybe_interrupt(CPUPPCState *env)
> {
> CPUState *cs = env_cpu(env);
> - bool locked = false;
> -
> - if (!qemu_mutex_iothread_locked()) {
> - locked = true;
> - qemu_mutex_lock_iothread();
> - }
> + QEMU_IOTHREAD_LOCK_GUARD();
>
> if (ppc_next_unmasked_interrupt(env)) {
> cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> } else {
> cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> -
> - if (locked) {
> - qemu_mutex_unlock_iothread();
> - }
> }
>
> #if defined(TARGET_PPC64)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb
2022-11-18 9:18 ` [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb Richard Henderson
@ 2022-11-18 10:13 ` Daniel Henrique Barboza
2022-11-18 10:35 ` Richard Henderson
2022-11-21 10:57 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 26+ messages in thread
From: Daniel Henrique Barboza @ 2022-11-18 10:13 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc
On 11/18/22 06:18, Richard Henderson wrote:
> In addition, use tcg_enabled instead of !kvm_enabled.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Should we strive for this change (tcg_enabled instead of !kvm_enabled)
everywhere when applicable? There's a lot of places in the ppc code where
this can be done.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: qemu-ppc@nongnu.org
> ---
> target/ppc/helper_regs.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index c0aee5855b..779e7db513 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -22,6 +22,7 @@
> #include "qemu/main-loop.h"
> #include "exec/exec-all.h"
> #include "sysemu/kvm.h"
> +#include "sysemu/tcg.h"
> #include "helper_regs.h"
> #include "power8-pmu.h"
> #include "cpu-models.h"
> @@ -203,17 +204,10 @@ void cpu_interrupt_exittb(CPUState *cs)
> {
> /*
> * We don't need to worry about translation blocks
> - * when running with KVM.
> + * unless running with TCG.
> */
> - if (kvm_enabled()) {
> - return;
> - }
> -
> - if (!qemu_mutex_iothread_locked()) {
> - qemu_mutex_lock_iothread();
> - cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> - qemu_mutex_unlock_iothread();
> - } else {
> + if (tcg_enabled()) {
> + QEMU_IOTHREAD_LOCK_GUARD();
> cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> }
> }
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq
2022-11-18 9:18 ` [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq Richard Henderson
@ 2022-11-18 10:14 ` Daniel Henrique Barboza
2022-11-21 10:57 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Daniel Henrique Barboza @ 2022-11-18 10:14 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc
On 11/18/22 06:18, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: qemu-ppc@nongnu.org
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> hw/ppc/ppc.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index dc86c1c7db..4e816c68c7 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -44,13 +44,9 @@ void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
> {
> CPUPPCState *env = &cpu->env;
> unsigned int old_pending;
> - bool locked = false;
>
> /* We may already have the BQL if coming from the reset path */
> - if (!qemu_mutex_iothread_locked()) {
> - locked = true;
> - qemu_mutex_lock_iothread();
> - }
> + QEMU_IOTHREAD_LOCK_GUARD();
>
> old_pending = env->pending_interrupts;
>
> @@ -67,10 +63,6 @@ void ppc_set_irq(PowerPCCPU *cpu, int irq, int level)
>
> trace_ppc_irq_set_exit(env, irq, level, env->pending_interrupts,
> CPU(cpu)->interrupt_request);
> -
> - if (locked) {
> - qemu_mutex_unlock_iothread();
> - }
> }
>
> /* PowerPC 6xx / 7xx internal IRQ controller */
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb
2022-11-18 10:13 ` Daniel Henrique Barboza
@ 2022-11-18 10:35 ` Richard Henderson
0 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 10:35 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel; +Cc: qemu-ppc
On 11/18/22 02:13, Daniel Henrique Barboza wrote:
>
>
> On 11/18/22 06:18, Richard Henderson wrote:
>> In addition, use tcg_enabled instead of !kvm_enabled.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>
> Should we strive for this change (tcg_enabled instead of !kvm_enabled)
> everywhere when applicable? There's a lot of places in the ppc code where
> this can be done.
I think it's better style, since that's generally what is meant.
It's important when the target supports multiple accelerators. A test for !kvm begs the
question of why we aren't also testing for e.g. !hvf. I've noticed a couple of these in
the code base.
r~
>
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>
>
>> Cc: qemu-ppc@nongnu.org
>> ---
>> target/ppc/helper_regs.c | 14 ++++----------
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
>> index c0aee5855b..779e7db513 100644
>> --- a/target/ppc/helper_regs.c
>> +++ b/target/ppc/helper_regs.c
>> @@ -22,6 +22,7 @@
>> #include "qemu/main-loop.h"
>> #include "exec/exec-all.h"
>> #include "sysemu/kvm.h"
>> +#include "sysemu/tcg.h"
>> #include "helper_regs.h"
>> #include "power8-pmu.h"
>> #include "cpu-models.h"
>> @@ -203,17 +204,10 @@ void cpu_interrupt_exittb(CPUState *cs)
>> {
>> /*
>> * We don't need to worry about translation blocks
>> - * when running with KVM.
>> + * unless running with TCG.
>> */
>> - if (kvm_enabled()) {
>> - return;
>> - }
>> -
>> - if (!qemu_mutex_iothread_locked()) {
>> - qemu_mutex_lock_iothread();
>> - cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
>> - qemu_mutex_unlock_iothread();
>> - } else {
>> + if (tcg_enabled()) {
>> + QEMU_IOTHREAD_LOCK_GUARD();
>> cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
>> }
>> }
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-18 9:18 ` [PATCH for-8.0 1/7] qemu/main-loop: " Richard Henderson
@ 2022-11-18 13:30 ` Alex Bennée
2022-11-20 23:30 ` Richard Henderson
2022-11-18 13:38 ` Alex Bennée
1 sibling, 1 reply; 26+ messages in thread
From: Alex Bennée @ 2022-11-18 13:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Create a wrapper for locking/unlocking the iothread lock.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:Main loop)
You might want to review Paolo's comments from:
Subject: [RFC PATCH] main-loop: introduce WITH_QEMU_IOTHREAD_LOCK
Date: Mon, 24 Oct 2022 18:19:09 +0100
Message-Id: <20221024171909.434818-1-alex.bennee@linaro.org>
So it would be worth having the WITH_QEMU_IOTHREAD_LOCK() and
MAYBE_WITH_QEMU_IOTHREAD_LOCK() helpers for completeness.
And of course the name cleanup.
> ---
> include/qemu/main-loop.h | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
> index 3c9a9a982d..c25f390696 100644
> --- a/include/qemu/main-loop.h
> +++ b/include/qemu/main-loop.h
> @@ -343,6 +343,35 @@ void qemu_mutex_lock_iothread_impl(const char *file, int line);
> */
> void qemu_mutex_unlock_iothread(void);
>
> +/**
> + * QEMU_IOTHREAD_LOCK_GUARD
> + *
> + * Wrap a block of code in a conditional qemu_mutex_{lock,unlock}_iothread.
> + */
> +typedef struct IOThreadLockAuto IOThreadLockAuto;
> +
> +static inline IOThreadLockAuto *qemu_iothread_auto_lock(const char *file,
> + int line)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + return NULL;
> + }
> + qemu_mutex_lock_iothread_impl(file, line);
> + /* Anything non-NULL causes the cleanup function to be called */
> + return (IOThreadLockAuto *)(uintptr_t)1;
> +}
> +
> +static inline void qemu_iothread_auto_unlock(IOThreadLockAuto *l)
> +{
> + qemu_mutex_unlock_iothread();
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(IOThreadLockAuto, qemu_iothread_auto_unlock)
> +
> +#define QEMU_IOTHREAD_LOCK_GUARD() \
> + g_autoptr(IOThreadLockAuto) _iothread_lock_auto __attribute__((unused)) \
> + = qemu_iothread_auto_lock(__FILE__, __LINE__)
> +
> /*
> * qemu_cond_wait_iothread: Wait on condition for the main loop mutex
> *
--
Alex Bennée
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-18 9:18 ` [PATCH for-8.0 1/7] qemu/main-loop: " Richard Henderson
2022-11-18 13:30 ` Alex Bennée
@ 2022-11-18 13:38 ` Alex Bennée
2022-11-18 18:22 ` Richard Henderson
1 sibling, 1 reply; 26+ messages in thread
From: Alex Bennée @ 2022-11-18 13:38 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Create a wrapper for locking/unlocking the iothread lock.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:Main loop)
> ---
> include/qemu/main-loop.h | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
> index 3c9a9a982d..c25f390696 100644
> --- a/include/qemu/main-loop.h
> +++ b/include/qemu/main-loop.h
> @@ -343,6 +343,35 @@ void qemu_mutex_lock_iothread_impl(const char *file, int line);
> */
> void qemu_mutex_unlock_iothread(void);
>
> +/**
> + * QEMU_IOTHREAD_LOCK_GUARD
> + *
> + * Wrap a block of code in a conditional qemu_mutex_{lock,unlock}_iothread.
> + */
> +typedef struct IOThreadLockAuto IOThreadLockAuto;
> +
> +static inline IOThreadLockAuto *qemu_iothread_auto_lock(const char *file,
> + int line)
> +{
> + if (qemu_mutex_iothread_locked()) {
> + return NULL;
> + }
> + qemu_mutex_lock_iothread_impl(file, line);
> + /* Anything non-NULL causes the cleanup function to be called */
> + return (IOThreadLockAuto *)(uintptr_t)1;
Oh hang on, what black magic is this. Does the compiler do a NULL check
before calling the cleanup?
> +}
> +
> +static inline void qemu_iothread_auto_unlock(IOThreadLockAuto *l)
> +{
> + qemu_mutex_unlock_iothread();
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(IOThreadLockAuto, qemu_iothread_auto_unlock)
> +
> +#define QEMU_IOTHREAD_LOCK_GUARD() \
> + g_autoptr(IOThreadLockAuto) _iothread_lock_auto __attribute__((unused)) \
> + = qemu_iothread_auto_lock(__FILE__, __LINE__)
> +
> /*
> * qemu_cond_wait_iothread: Wait on condition for the main loop mutex
> *
--
Alex Bennée
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-18 13:38 ` Alex Bennée
@ 2022-11-18 18:22 ` Richard Henderson
0 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2022-11-18 18:22 UTC (permalink / raw)
To: Alex Bennée; +Cc: Paolo Bonzini, qemu-devel
On 11/18/22 05:38, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Create a wrapper for locking/unlocking the iothread lock.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:Main loop)
>> ---
>> include/qemu/main-loop.h | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
>> index 3c9a9a982d..c25f390696 100644
>> --- a/include/qemu/main-loop.h
>> +++ b/include/qemu/main-loop.h
>> @@ -343,6 +343,35 @@ void qemu_mutex_lock_iothread_impl(const char *file, int line);
>> */
>> void qemu_mutex_unlock_iothread(void);
>>
>> +/**
>> + * QEMU_IOTHREAD_LOCK_GUARD
>> + *
>> + * Wrap a block of code in a conditional qemu_mutex_{lock,unlock}_iothread.
>> + */
>> +typedef struct IOThreadLockAuto IOThreadLockAuto;
>> +
>> +static inline IOThreadLockAuto *qemu_iothread_auto_lock(const char *file,
>> + int line)
>> +{
>> + if (qemu_mutex_iothread_locked()) {
>> + return NULL;
>> + }
>> + qemu_mutex_lock_iothread_impl(file, line);
>> + /* Anything non-NULL causes the cleanup function to be called */
>> + return (IOThreadLockAuto *)(uintptr_t)1;
>
> Oh hang on, what black magic is this. Does the compiler do a NULL check
> before calling the cleanup?
Not the compiler, but...
>> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(IOThreadLockAuto, qemu_iothread_auto_unlock)
... this does. Follow the macros down and you get
> static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) (TypeName *_ptr)
> { if (_ptr) (cleanup) ((ParentName *) _ptr); }
r~
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip
2022-11-18 9:18 ` [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip Richard Henderson
@ 2022-11-20 23:18 ` Alistair Francis
2022-11-21 10:56 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-11-20 23:18 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, Alistair Francis, Bin Meng, qemu-riscv
On Fri, Nov 18, 2022 at 7:22 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Bin Meng <bin.meng@windriver.com>
> Cc: qemu-riscv@nongnu.org
> ---
> target/riscv/cpu_helper.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 278d163803..241d06bab8 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -610,7 +610,6 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value)
> CPURISCVState *env = &cpu->env;
> CPUState *cs = CPU(cpu);
> uint64_t gein, vsgein = 0, vstip = 0, old = env->mip;
> - bool locked = false;
>
> if (riscv_cpu_virt_enabled(env)) {
> gein = get_field(env->hstatus, HSTATUS_VGEIN);
> @@ -621,10 +620,7 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value)
> mask = ((mask == MIP_VSTIP) && env->vstime_irq) ? 0 : mask;
> vstip = env->vstime_irq ? MIP_VSTIP : 0;
>
> - if (!qemu_mutex_iothread_locked()) {
> - locked = true;
> - qemu_mutex_lock_iothread();
> - }
> + QEMU_IOTHREAD_LOCK_GUARD();
>
> env->mip = (env->mip & ~mask) | (value & mask);
>
> @@ -634,10 +630,6 @@ uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value)
> cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> }
>
> - if (locked) {
> - qemu_mutex_unlock_iothread();
> - }
> -
> return old;
> }
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-18 13:30 ` Alex Bennée
@ 2022-11-20 23:30 ` Richard Henderson
2022-11-21 9:55 ` Alex Bennée
0 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2022-11-20 23:30 UTC (permalink / raw)
To: Alex Bennée; +Cc: Paolo Bonzini, qemu-devel
On 11/18/22 05:30, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Create a wrapper for locking/unlocking the iothread lock.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:Main loop)
>
> You might want to review Paolo's comments from:
>
> Subject: [RFC PATCH] main-loop: introduce WITH_QEMU_IOTHREAD_LOCK
> Date: Mon, 24 Oct 2022 18:19:09 +0100
> Message-Id: <20221024171909.434818-1-alex.bennee@linaro.org>
>
> So it would be worth having the WITH_QEMU_IOTHREAD_LOCK() and
> MAYBE_WITH_QEMU_IOTHREAD_LOCK() helpers for completeness.
I don't see that (MAYBE_)WITH_QEMU_IOTHREAD_LOCK is particularly useful in any of the
cases that I converted.
> And of course the name cleanup.
What name cleanup?
r~
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-20 23:30 ` Richard Henderson
@ 2022-11-21 9:55 ` Alex Bennée
2022-11-21 11:05 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 26+ messages in thread
From: Alex Bennée @ 2022-11-21 9:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> On 11/18/22 05:30, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> Create a wrapper for locking/unlocking the iothread lock.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:Main loop)
>> You might want to review Paolo's comments from:
>> Subject: [RFC PATCH] main-loop: introduce WITH_QEMU_IOTHREAD_LOCK
>> Date: Mon, 24 Oct 2022 18:19:09 +0100
>> Message-Id: <20221024171909.434818-1-alex.bennee@linaro.org>
>> So it would be worth having the WITH_QEMU_IOTHREAD_LOCK() and
>> MAYBE_WITH_QEMU_IOTHREAD_LOCK() helpers for completeness.
>
> I don't see that (MAYBE_)WITH_QEMU_IOTHREAD_LOCK is particularly
> useful in any of the cases that I converted.
Fair enough - as long as they are easy enough to add later. The WITH_
forms do work nicely to wrap a particular area under lock and make
things visually clear vs the LOCK_GUARD which basically holds the lock
to the end of function or exit.
>
>> And of course the name cleanup.
>
> What name cleanup?
Also lots of bonus points for finally renaming these functions to
"*_main_thread" rather than "*_iothread" since, confusingly, iothreads
(plural) are the only ones that do not and cannot take the "iothread
lock".
>
>
> r~
--
Alex Bennée
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 2/7] hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request
2022-11-18 9:18 ` [PATCH for-8.0 2/7] hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request Richard Henderson
@ 2022-11-21 10:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 10:55 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Jiaxun Yang
On 18/11/22 10:18, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> hw/mips/mips_int.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
> index 2db5e10fe0..73437cd90f 100644
> --- a/hw/mips/mips_int.c
> +++ b/hw/mips/mips_int.c
> @@ -32,17 +32,12 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
> MIPSCPU *cpu = opaque;
> CPUMIPSState *env = &cpu->env;
> CPUState *cs = CPU(cpu);
> - bool locked = false;
>
> if (irq < 0 || irq > 7) {
> return;
> }
>
> - /* Make sure locking works even if BQL is already held by the caller */
> - if (!qemu_mutex_iothread_locked()) {
> - locked = true;
> - qemu_mutex_lock_iothread();
> - }
> + QEMU_IOTHREAD_LOCK_GUARD();
>
> if (level) {
> env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
> @@ -59,10 +54,6 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
> } else {
> cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> }
> -
> - if (locked) {
> - qemu_mutex_unlock_iothread();
> - }
> }
I'd rather have a macro similar to WITH_RCU_READ_LOCK_GUARD()
so the locked context is obvious:
WITH_QEMU_IOTHREAD_LOCK_GUARD() {
...
}
Anyhow:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt
2022-11-18 9:18 ` [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt Richard Henderson
2022-11-18 10:12 ` Daniel Henrique Barboza
@ 2022-11-21 10:56 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 10:56 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc
On 18/11/22 10:18, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: qemu-ppc@nongnu.org
> ---
> target/ppc/excp_helper.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip
2022-11-18 9:18 ` [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip Richard Henderson
2022-11-20 23:18 ` Alistair Francis
@ 2022-11-21 10:56 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 10:56 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Alistair Francis, Bin Meng, qemu-riscv
On 18/11/22 10:18, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Bin Meng <bin.meng@windriver.com>
> Cc: qemu-riscv@nongnu.org
> ---
> target/riscv/cpu_helper.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb
2022-11-18 9:18 ` [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb Richard Henderson
2022-11-18 10:13 ` Daniel Henrique Barboza
@ 2022-11-21 10:57 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 10:57 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc
On 18/11/22 10:18, Richard Henderson wrote:
> In addition, use tcg_enabled instead of !kvm_enabled.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: qemu-ppc@nongnu.org
> ---
> target/ppc/helper_regs.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq
2022-11-18 9:18 ` [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq Richard Henderson
2022-11-18 10:14 ` Daniel Henrique Barboza
@ 2022-11-21 10:57 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 10:57 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc
On 18/11/22 10:18, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: qemu-ppc@nongnu.org
> ---
> hw/ppc/ppc.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex
2022-11-18 9:18 ` [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex Richard Henderson
@ 2022-11-21 11:02 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 11:02 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 18/11/22 10:18, Richard Henderson wrote:
> Narrow the scope of the lock to the actual read/write,
> moving the cpu_transation_failed call outside the lock.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> accel/tcg/cputlb.c | 25 ++++++++-----------------
> 1 file changed, 8 insertions(+), 17 deletions(-)
> @@ -1367,11 +1366,11 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full,
> cpu_io_recompile(cpu, retaddr);
> }
>
> - if (!qemu_mutex_iothread_locked()) {
> - qemu_mutex_lock_iothread();
> - locked = true;
> + {
> + QEMU_IOTHREAD_LOCK_GUARD();
> + r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
> }
> - r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
> +
Example of clearer WITH_QEMU_IOTHREAD_LOCK_GUARD() macro
use suggested earlier:
WITH_QEMU_IOTHREAD_LOCK_GUARD() {
r = memory_region_dispatch_read(mr, mr_offset, &val,
op, full->attrs);
}
> if (r != MEMTX_OK) {
> hwaddr physaddr = mr_offset +
> section->offset_within_address_space -
> @@ -1380,10 +1379,6 @@ static uint64_t io_readx(CPUArchState *env, CPUTLBEntryFull *full,
> cpu_transaction_failed(cpu, physaddr, addr, memop_size(op), access_type,
> mmu_idx, full->attrs, r, retaddr);
> }
> - if (locked) {
> - qemu_mutex_unlock_iothread();
> - }
> -
> return val;
> }
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
2022-11-21 9:55 ` Alex Bennée
@ 2022-11-21 11:05 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-21 11:05 UTC (permalink / raw)
To: Alex Bennée, Richard Henderson; +Cc: Paolo Bonzini, qemu-devel
On 21/11/22 10:55, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> On 11/18/22 05:30, Alex Bennée wrote:
>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>
>>>> Create a wrapper for locking/unlocking the iothread lock.
>>>>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:Main loop)
>>> You might want to review Paolo's comments from:
>>> Subject: [RFC PATCH] main-loop: introduce WITH_QEMU_IOTHREAD_LOCK
>>> Date: Mon, 24 Oct 2022 18:19:09 +0100
>>> Message-Id: <20221024171909.434818-1-alex.bennee@linaro.org>
>>> So it would be worth having the WITH_QEMU_IOTHREAD_LOCK() and
>>> MAYBE_WITH_QEMU_IOTHREAD_LOCK() helpers for completeness.
>>
>> I don't see that (MAYBE_)WITH_QEMU_IOTHREAD_LOCK is particularly
>> useful in any of the cases that I converted.
>
> Fair enough - as long as they are easy enough to add later. The WITH_
> forms do work nicely to wrap a particular area under lock and make
> things visually clear vs the LOCK_GUARD which basically holds the lock
> to the end of function or exit.
I concur for WITH_QEMU_IOTHREAD_LOCK(), it is a no-brainer.
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2022-11-21 11:05 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-18 9:18 [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Richard Henderson
2022-11-18 9:18 ` [PATCH for-8.0 1/7] qemu/main-loop: " Richard Henderson
2022-11-18 13:30 ` Alex Bennée
2022-11-20 23:30 ` Richard Henderson
2022-11-21 9:55 ` Alex Bennée
2022-11-21 11:05 ` Philippe Mathieu-Daudé
2022-11-18 13:38 ` Alex Bennée
2022-11-18 18:22 ` Richard Henderson
2022-11-18 9:18 ` [PATCH for-8.0 2/7] hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request Richard Henderson
2022-11-21 10:55 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt Richard Henderson
2022-11-18 10:12 ` Daniel Henrique Barboza
2022-11-21 10:56 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb Richard Henderson
2022-11-18 10:13 ` Daniel Henrique Barboza
2022-11-18 10:35 ` Richard Henderson
2022-11-21 10:57 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip Richard Henderson
2022-11-20 23:18 ` Alistair Francis
2022-11-21 10:56 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq Richard Henderson
2022-11-18 10:14 ` Daniel Henrique Barboza
2022-11-21 10:57 ` Philippe Mathieu-Daudé
2022-11-18 9:18 ` [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex Richard Henderson
2022-11-21 11:02 ` Philippe Mathieu-Daudé
2022-11-18 9:36 ` [PATCH for-8.0 0/7] main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD Paolo Bonzini
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.