diff mbox series

powerpc/pseries: unregister VPA when hot unplugging a CPU

Message ID 20221114160150.13554-1-ldufour@linux.ibm.com (mailing list archive)
State Accepted
Commit f6aa37c51ec0d053ee34c235bfe0e666618a3baf
Headers show
Series powerpc/pseries: unregister VPA when hot unplugging a CPU | expand

Checks

Context Check Description
snowpatch_ozlabs/github-powerpc_selftests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_ppctests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_sparse success Successfully ran 4 jobs.
snowpatch_ozlabs/github-powerpc_clang success Successfully ran 6 jobs.

Commit Message

Laurent Dufour Nov. 14, 2022, 4:01 p.m. UTC
The VPA should unregister when offlining a CPU. Otherwise there could be a
short window where 2 CPUs could share the same VPA.

This happens because the hypervisor is still keeping the VPA attached to
the vCPU even if it became offline.

Here is a potential situation:
 1. remove proc A,
 2. add proc B. If proc B gets proc A's place in cpu_present_map, then it
    registers proc A's VPAs.
 3. If proc B is then re-added to the LP, its threads are sharing VPAs with
    proc A briefly as they come online.

As the hypervisor may check for the VPA's yield_count field oddity, it may
detects an unexpected value and kill the LPAR.

Suggested-by: Nathan Lynch <nathanl@linux.ibm.com>
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Nathan Lynch Nov. 18, 2022, 5:52 p.m. UTC | #1
Laurent Dufour <ldufour@linux.ibm.com> writes:

> The VPA should unregister when offlining a CPU. Otherwise there could be a
> short window where 2 CPUs could share the same VPA.
>
> This happens because the hypervisor is still keeping the VPA attached to
> the vCPU even if it became offline.
>
> Here is a potential situation:
>  1. remove proc A,
>  2. add proc B. If proc B gets proc A's place in cpu_present_map, then it
>     registers proc A's VPAs.
>  3. If proc B is then re-added to the LP, its threads are sharing VPAs with
>     proc A briefly as they come online.
>
> As the hypervisor may check for the VPA's yield_count field oddity, it may
> detects an unexpected value and kill the LPAR.
>
> Suggested-by: Nathan Lynch <nathanl@linux.ibm.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index e0a7ac5db15d..090ae5a1e0f5 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -70,6 +70,7 @@ static void pseries_cpu_offline_self(void)
>  		xics_teardown_cpu();
>  
>  	unregister_slb_shadow(hwcpu);
> +	unregister_vpa(hwcpu);
>  	rtas_stop_self();
>  
>  	/* Should never get here... */

Reviewed-by: Nathan Lynch <nathanl@linux.ibm.com>

I was wondering whether we could leave an active dispatch trace log
buffer registered, which could interfere with releasing the VPA, but I
verified that DTL has the appropriate cpuhp callback for that
(dtl_worker_offline()).

Alternatively we could change the code to dynamically register and
unregister VPAs only on processor add and remove, as opposed to CPU
online/offline. But I can't see any significant advantage to that.
Michael Ellerman Dec. 8, 2022, 12:40 p.m. UTC | #2
On Mon, 14 Nov 2022 17:01:50 +0100, Laurent Dufour wrote:
> The VPA should unregister when offlining a CPU. Otherwise there could be a
> short window where 2 CPUs could share the same VPA.
> 
> This happens because the hypervisor is still keeping the VPA attached to
> the vCPU even if it became offline.
> 
> Here is a potential situation:
>  1. remove proc A,
>  2. add proc B. If proc B gets proc A's place in cpu_present_map, then it
>     registers proc A's VPAs.
>  3. If proc B is then re-added to the LP, its threads are sharing VPAs with
>     proc A briefly as they come online.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/pseries: unregister VPA when hot unplugging a CPU
      https://git.kernel.org/powerpc/c/f6aa37c51ec0d053ee34c235bfe0e666618a3baf

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index e0a7ac5db15d..090ae5a1e0f5 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -70,6 +70,7 @@  static void pseries_cpu_offline_self(void)
 		xics_teardown_cpu();
 
 	unregister_slb_shadow(hwcpu);
+	unregister_vpa(hwcpu);
 	rtas_stop_self();
 
 	/* Should never get here... */