Re: [PATCH v3 4/7] x86/hyper-v: allocate and use Virtual Processor Assist Pages

[Thomas Gleixner]

On Fri, 9 Mar 2018, Vitaly Kuznetsov wrote:
> @@ -198,6 +218,12 @@ static int hv_cpu_die(unsigned int cpu)
>  	struct hv_reenlightenment_control re_ctrl;
>  	unsigned int new_cpu;
>  
> +	if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
> +		wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> +		vfree(hv_vp_assist_page[cpu]);
> +		hv_vp_assist_page[cpu] = NULL;

So this is freed before the CPU is actually dead. And this runs in
preemtible context. Is the wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0); enough to
prevent eventual users of the assist page on the outgoing CPU from
accessing it?

>  	if (hv_reenlightenment_cb == NULL)
>  		return 0;
>  
> @@ -241,6 +267,13 @@ void hyperv_init(void)
>  	if (!hv_vp_index)
>  		return;
>  
> +	hv_vp_assist_page = kcalloc(num_possible_cpus(),
> +				    sizeof(*hv_vp_assist_page), GFP_KERNEL);
> +	if (!hv_vp_assist_page) {
> +		ms_hyperv.hints &= ~HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
> +		return;
> +	}
> +
>  	if (cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv_init:online",
>  			      hv_cpu_init, hv_cpu_die) < 0)
>  		goto free_vp_index;

Shouldn't you free hv_vp_assist_page in the error path?

> +extern struct hv_vp_assist_page **hv_vp_assist_page;
> +
> +static inline struct hv_vp_assist_page *hv_get_vp_assist_page(unsigned int cpu)
> +{
> +	return hv_vp_assist_page[cpu];

Shouldn't that check hv_vp_assist_page != NULL?

Thanks,

	tglx