* Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
2026-09-23 12:14 [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize Shivang Upadhyay
@ 2026-09-23 13:39 ` Philippe Mathieu-Daudé
2026-09-24 13:46 ` Shivang Upadhyay
2026-09-24 9:12 ` Mukesh Kumar Chaurasiya
2026-09-24 14:42 ` Amit Machhiwal
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-09-23 13:39 UTC (permalink / raw)
To: Shivang Upadhyay, qemu-ppc, qemu-devel
Cc: sourabhjain, rathc, npiggin, milesg, harshpb, adityag,
richard.henderson, mkchauras, sshegde, srikar, amachhiw
Hi,
On 23/9/26 14:14, Shivang Upadhyay wrote:
> During CPU hot-unplug (e.g. via dynamic reconfiguration unplug),
> ppc_cpu_unrealize() invoked pcc->parent_unrealize(dev) before calling
> cpu_remove_sync(CPU(cpu)).
>
> pcc->parent_unrealize() calls cpu_common_unrealize(), which triggers
> accel_cpu_common_unrealize() -> tcg_exec_unrealizefn() -> tlb_destroy().
> This immediately frees the CPU's TLB tables and structures. Because the
> vCPU thread had not yet been stopped and joined via cpu_remove_sync(),
> the vCPU thread was still actively running its event loop and processing
> queued CPU work (such as tcg_commit_cpu / tlb_flush).
>
> This resulted in a race where the running vCPU thread accessed and freed
> already-destroyed TLB tables concurrently with tlb_destroy(), leading to
> Segfault (due to heap corruption).
>
> AddressSanitizer build reported a double-free:
>
> =================================================================
> ==121930==ERROR: AddressSanitizer: attempting double-free on 0x7ef8f3438800 in thread T14:
> #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
> #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
> #2 0x558bf6a391b1 in tlb_mmu_resize_locked accel/tcg/cputlb.c:249
> #3 0x558bf6a396b5 in tlb_flush_one_mmuidx_locked accel/tcg/cputlb.c:296
> #4 0x558bf6a39f91 in tlb_flush_by_mmuidx_async_work accel/tcg/cputlb.c:390
> #5 0x558bf6a3a200 in tlb_flush_by_mmuidx accel/tcg/cputlb.c:417
> #6 0x558bf6a3a22a in tlb_flush accel/tcg/cputlb.c:422
> #7 0x558bf73f31ac in tcg_commit_cpu system/physmem.c:3068
> #8 0x558bf6987c55 in process_queued_cpu_work cpu-common.c:378
> #9 0x558bf73a9913 in qemu_process_cpu_events_common system/cpus.c:402
> #10 0x558bf73a9a46 in qemu_process_cpu_events system/cpus.c:421
> #11 0x558bf6a65974 in mttcg_cpu_thread_fn accel/tcg/tcg-accel-ops-mttcg.c:90
>
> 0x7ef8f3438800 is located 0 bytes inside of 65536-byte region [0x7ef8f3438800,0x7ef8f3448800)
> freed by thread T9 here:
> #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
> #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
> #2 0x558bf6a39a91 in tlb_destroy accel/tcg/cputlb.c:345
> #3 0x558bf6a16354 in tcg_exec_unrealizefn accel/tcg/cpu-exec.c:1094
> #4 0x558bf693d073 in accel_cpu_common_unrealize accel/accel-common.c:117
> #5 0x558bf6980e37 in cpu_common_unrealize hw/core/cpu-common.c:279
> #6 0x558bf6980dfa in cpu_common_unrealizefn hw/core/cpu-common.c:267
> #7 0x558bf763ef65 in ppc_cpu_unrealize target/ppc/cpu_init.c:6965
> #8 0x558bf7872199 in device_set_realized hw/core/qdev.c:618
> #14 0x558bf756068f in spapr_unrealize_vcpu hw/ppc/spapr_cpu_core.c:209
>
> Fix this by moving cpu_remove_sync() before pcc->parent_unrealize(dev)
> in ppc_cpu_unrealize(), ensuring the vCPU thread is stopped, has
> finished processing its events, and is joined before CPU resources
> and accelerator state are destroyed.
>
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
> target/ppc/cpu_init.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Good enough for now:
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
But long term we should move the call to cpu_common_unrealize() IMO,
it shouldn't be called per-target.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
2026-09-23 13:39 ` Philippe Mathieu-Daudé
@ 2026-09-24 13:46 ` Shivang Upadhyay
0 siblings, 0 replies; 7+ messages in thread
From: Shivang Upadhyay @ 2026-09-24 13:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-ppc, qemu-devel
Cc: sourabhjain, rathc, npiggin, milesg, harshpb, adityag,
richard.henderson, mkchauras, sshegde, srikar, amachhiw
On Wed, 2026-09-23 at 15:39 +0200, Philippe Mathieu-Daudé wrote:
> > Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> > ---
> > target/ppc/cpu_init.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Good enough for now:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>
Thanks Philippe.
> But long term we should move the call to cpu_common_unrealize() IMO,
> it shouldn't be called per-target.
Good suggestion, As this should be same for all archs.
~Shivang.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
2026-09-23 12:14 [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize Shivang Upadhyay
2026-09-23 13:39 ` Philippe Mathieu-Daudé
@ 2026-09-24 9:12 ` Mukesh Kumar Chaurasiya
2026-09-24 13:18 ` Shivang Upadhyay
2026-09-24 14:42 ` Amit Machhiwal
2 siblings, 1 reply; 7+ messages in thread
From: Mukesh Kumar Chaurasiya @ 2026-09-24 9:12 UTC (permalink / raw)
To: Shivang Upadhyay
Cc: qemu-ppc, qemu-devel, sourabhjain, rathc, npiggin, milesg,
harshpb, adityag, richard.henderson, sshegde, srikar, amachhiw
On Wed, Sep 23, 2026 at 05:44:44PM +0530, Shivang Upadhyay wrote:
> During CPU hot-unplug (e.g. via dynamic reconfiguration unplug),
> ppc_cpu_unrealize() invoked pcc->parent_unrealize(dev) before calling
> cpu_remove_sync(CPU(cpu)).
>
> pcc->parent_unrealize() calls cpu_common_unrealize(), which triggers
> accel_cpu_common_unrealize() -> tcg_exec_unrealizefn() -> tlb_destroy().
> This immediately frees the CPU's TLB tables and structures. Because the
> vCPU thread had not yet been stopped and joined via cpu_remove_sync(),
> the vCPU thread was still actively running its event loop and processing
> queued CPU work (such as tcg_commit_cpu / tlb_flush).
>
> This resulted in a race where the running vCPU thread accessed and freed
> already-destroyed TLB tables concurrently with tlb_destroy(), leading to
> Segfault (due to heap corruption).
>
> AddressSanitizer build reported a double-free:
>
> =================================================================
> ==121930==ERROR: AddressSanitizer: attempting double-free on 0x7ef8f3438800 in thread T14:
> #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
> #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
> #2 0x558bf6a391b1 in tlb_mmu_resize_locked accel/tcg/cputlb.c:249
> #3 0x558bf6a396b5 in tlb_flush_one_mmuidx_locked accel/tcg/cputlb.c:296
> #4 0x558bf6a39f91 in tlb_flush_by_mmuidx_async_work accel/tcg/cputlb.c:390
> #5 0x558bf6a3a200 in tlb_flush_by_mmuidx accel/tcg/cputlb.c:417
> #6 0x558bf6a3a22a in tlb_flush accel/tcg/cputlb.c:422
> #7 0x558bf73f31ac in tcg_commit_cpu system/physmem.c:3068
> #8 0x558bf6987c55 in process_queued_cpu_work cpu-common.c:378
> #9 0x558bf73a9913 in qemu_process_cpu_events_common system/cpus.c:402
> #10 0x558bf73a9a46 in qemu_process_cpu_events system/cpus.c:421
> #11 0x558bf6a65974 in mttcg_cpu_thread_fn accel/tcg/tcg-accel-ops-mttcg.c:90
>
> 0x7ef8f3438800 is located 0 bytes inside of 65536-byte region [0x7ef8f3438800,0x7ef8f3448800)
> freed by thread T9 here:
> #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
> #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
> #2 0x558bf6a39a91 in tlb_destroy accel/tcg/cputlb.c:345
> #3 0x558bf6a16354 in tcg_exec_unrealizefn accel/tcg/cpu-exec.c:1094
> #4 0x558bf693d073 in accel_cpu_common_unrealize accel/accel-common.c:117
> #5 0x558bf6980e37 in cpu_common_unrealize hw/core/cpu-common.c:279
> #6 0x558bf6980dfa in cpu_common_unrealizefn hw/core/cpu-common.c:267
> #7 0x558bf763ef65 in ppc_cpu_unrealize target/ppc/cpu_init.c:6965
> #8 0x558bf7872199 in device_set_realized hw/core/qdev.c:618
> #14 0x558bf756068f in spapr_unrealize_vcpu hw/ppc/spapr_cpu_core.c:209
>
> Fix this by moving cpu_remove_sync() before pcc->parent_unrealize(dev)
> in ppc_cpu_unrealize(), ensuring the vCPU thread is stopped, has
> finished processing its events, and is joined before CPU resources
> and accelerator state are destroyed.
>
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
> target/ppc/cpu_init.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 6c626843c9..b711f9c0a8 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6962,10 +6962,10 @@ static void ppc_cpu_unrealize(DeviceState *dev)
> PowerPCCPU *cpu = POWERPC_CPU(dev);
> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>
> - pcc->parent_unrealize(dev);
> -
> cpu_remove_sync(CPU(cpu));
>
> + pcc->parent_unrealize(dev);
> +
> destroy_ppc_opcodes(cpu);
> }
>
> --
> 2.54.0
>
Makes Sense.
Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
2026-09-24 9:12 ` Mukesh Kumar Chaurasiya
@ 2026-09-24 13:18 ` Shivang Upadhyay
0 siblings, 0 replies; 7+ messages in thread
From: Shivang Upadhyay @ 2026-09-24 13:18 UTC (permalink / raw)
To: Mukesh Kumar Chaurasiya
Cc: qemu-ppc, qemu-devel, sourabhjain, rathc, npiggin, milesg,
harshpb, adityag, richard.henderson, sshegde, srikar, amachhiw
On Thu, 2026-09-24 at 14:42 +0530, Mukesh Kumar Chaurasiya wrote:
> Makes Sense.
>
> Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
Thanks for your review Mukesh :)
~Shivang.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
2026-09-23 12:14 [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize Shivang Upadhyay
2026-09-23 13:39 ` Philippe Mathieu-Daudé
2026-09-24 9:12 ` Mukesh Kumar Chaurasiya
@ 2026-09-24 14:42 ` Amit Machhiwal
2026-09-25 8:00 ` Shivang Upadhyay
2 siblings, 1 reply; 7+ messages in thread
From: Amit Machhiwal @ 2026-09-24 14:42 UTC (permalink / raw)
To: Shivang Upadhyay
Cc: qemu-ppc, qemu-devel, sourabhjain, rathc, npiggin, milesg,
harshpb, adityag, richard.henderson, mkchauras, sshegde, srikar,
amachhiw
On 2026/09/23 05:44 PM, Shivang Upadhyay wrote:
> During CPU hot-unplug (e.g. via dynamic reconfiguration unplug),
> ppc_cpu_unrealize() invoked pcc->parent_unrealize(dev) before calling
> cpu_remove_sync(CPU(cpu)).
>
> pcc->parent_unrealize() calls cpu_common_unrealize(), which triggers
> accel_cpu_common_unrealize() -> tcg_exec_unrealizefn() -> tlb_destroy().
> This immediately frees the CPU's TLB tables and structures. Because the
> vCPU thread had not yet been stopped and joined via cpu_remove_sync(),
> the vCPU thread was still actively running its event loop and processing
> queued CPU work (such as tcg_commit_cpu / tlb_flush).
>
> This resulted in a race where the running vCPU thread accessed and freed
> already-destroyed TLB tables concurrently with tlb_destroy(), leading to
> Segfault (due to heap corruption).
>
> AddressSanitizer build reported a double-free:
>
> =================================================================
> ==121930==ERROR: AddressSanitizer: attempting double-free on 0x7ef8f3438800 in thread T14:
> #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
> #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
> #2 0x558bf6a391b1 in tlb_mmu_resize_locked accel/tcg/cputlb.c:249
> #3 0x558bf6a396b5 in tlb_flush_one_mmuidx_locked accel/tcg/cputlb.c:296
> #4 0x558bf6a39f91 in tlb_flush_by_mmuidx_async_work accel/tcg/cputlb.c:390
> #5 0x558bf6a3a200 in tlb_flush_by_mmuidx accel/tcg/cputlb.c:417
> #6 0x558bf6a3a22a in tlb_flush accel/tcg/cputlb.c:422
> #7 0x558bf73f31ac in tcg_commit_cpu system/physmem.c:3068
> #8 0x558bf6987c55 in process_queued_cpu_work cpu-common.c:378
> #9 0x558bf73a9913 in qemu_process_cpu_events_common system/cpus.c:402
> #10 0x558bf73a9a46 in qemu_process_cpu_events system/cpus.c:421
> #11 0x558bf6a65974 in mttcg_cpu_thread_fn accel/tcg/tcg-accel-ops-mttcg.c:90
>
> 0x7ef8f3438800 is located 0 bytes inside of 65536-byte region [0x7ef8f3438800,0x7ef8f3448800)
> freed by thread T9 here:
> #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
> #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
> #2 0x558bf6a39a91 in tlb_destroy accel/tcg/cputlb.c:345
> #3 0x558bf6a16354 in tcg_exec_unrealizefn accel/tcg/cpu-exec.c:1094
> #4 0x558bf693d073 in accel_cpu_common_unrealize accel/accel-common.c:117
> #5 0x558bf6980e37 in cpu_common_unrealize hw/core/cpu-common.c:279
> #6 0x558bf6980dfa in cpu_common_unrealizefn hw/core/cpu-common.c:267
> #7 0x558bf763ef65 in ppc_cpu_unrealize target/ppc/cpu_init.c:6965
> #8 0x558bf7872199 in device_set_realized hw/core/qdev.c:618
> #14 0x558bf756068f in spapr_unrealize_vcpu hw/ppc/spapr_cpu_core.c:209
>
> Fix this by moving cpu_remove_sync() before pcc->parent_unrealize(dev)
> in ppc_cpu_unrealize(), ensuring the vCPU thread is stopped, has
> finished processing its events, and is joined before CPU resources
> and accelerator state are destroyed.
>
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
> target/ppc/cpu_init.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 6c626843c9..b711f9c0a8 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6962,10 +6962,10 @@ static void ppc_cpu_unrealize(DeviceState *dev)
> PowerPCCPU *cpu = POWERPC_CPU(dev);
> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>
> - pcc->parent_unrealize(dev);
> -
> cpu_remove_sync(CPU(cpu));
>
> + pcc->parent_unrealize(dev);
> +
The fix is correct and the commit message is well written. The ASan trace makes
the race unambiguous: tlb_destroy() frees fast->table and desc->fulltlb while
the MTTCG thread is still running tlb_mmu_resize_locked() on the same structures
via the queued tcg_commit_cpu work.
Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>
Minor: I think a short comment above cpu_remove_sync() would make the ordering
constraint self-documenting, e.g.:
/*
* Stop and join the vCPU thread before tearing down accelerator
* state (TLB tables etc.) to prevent the running thread from
* accessing freed resources.
*/
cpu_remove_sync(CPU(cpu));
Thanks,
Amit
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
2026-09-24 14:42 ` Amit Machhiwal
@ 2026-09-25 8:00 ` Shivang Upadhyay
0 siblings, 0 replies; 7+ messages in thread
From: Shivang Upadhyay @ 2026-09-25 8:00 UTC (permalink / raw)
To: qemu-ppc, qemu-devel, sourabhjain, rathc, npiggin, milesg,
harshpb, adityag, richard.henderson, mkchauras, sshegde, srikar
On Thu, 2026-09-24 at 20:12 +0530, Amit Machhiwal wrote:
> The fix is correct and the commit message is well written. The ASan
> trace makes
> the race unambiguous: tlb_destroy() frees fast->table and desc-
> >fulltlb while
> the MTTCG thread is still running tlb_mmu_resize_locked() on the same
> structures
> via the queued tcg_commit_cpu work.
>
> Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>
>
Thanks for review.
> Minor: I think a short comment above cpu_remove_sync() would make the
> ordering
> constraint self-documenting, e.g.:
>
> /*
> * Stop and join the vCPU thread before tearing down accelerator
> * state (TLB tables etc.) to prevent the running thread from
> * accessing freed resources.
> */
> cpu_remove_sync(CPU(cpu));
Seems this is already pulled via misc tree.
I plan to work on Philippe's suggestion to pull all cpu_remove_sync()
calls into cpu_common_unrealize(), that should take care of all such
caveats.
~Shivang.
^ permalink raw reply [flat|nested] 7+ messages in thread