Re: [PATCH] powerpc/pseries: unregister VPA when hot unplugging a CPU

[Nathan Lynch <nathanl@linux.ibm.com>]

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.