RE: [PATCH v2] x86/hyper-v: Zero out the VP ASSIST PAGE to fix CPU offlining

[Michael Kelley <mikelley@microsoft.com>]

From: Dexuan Cui <decui@microsoft.com> Sent: Thursday, July 18, 2019 8:23 PM
> 
> The VP ASSIST PAGE is an "overlay" page (see Hyper-V TLFS's Section
> 5.2.1 "GPA Overlay Pages" for the details) and here is an excerpt:
> 
> "
> The hypervisor defines several special pages that "overlay" the guest's
> Guest Physical Addresses (GPA) space. Overlays are addressed GPA but are
> not included in the normal GPA map maintained internally by the hypervisor.
> Conceptually, they exist in a separate map that overlays the GPA map.
> 
> If a page within the GPA space is overlaid, any SPA page mapped to the
> GPA page is effectively "obscured" and generally unreachable by the
> virtual processor through processor memory accesses.
> 
> If an overlay page is disabled, the underlying GPA page is "uncovered",
> and an existing mapping becomes accessible to the guest.
> "
> 
> SPA = System Physical Address = the final real physical address.
> 
> When a CPU (e.g. CPU1) is being onlined, in hv_cpu_init(), we allocate the
> VP ASSIST PAGE and enable the EOI optimization for this CPU by writing the
> MSR HV_X64_MSR_VP_ASSIST_PAGE. From now on, hvp->apic_assist belongs to the
> special SPA page, and this CPU *always* uses hvp->apic_assist (which is
> shared with the hypervisor) to decide if it needs to write the EOI MSR.
> 
> When a CPU (e.g. CPU1) is being offlined, on this CPU, we do:
> 1. in hv_cpu_die(), we disable the EOI optimizaton for this CPU, and from
>    now on hvp->apic_assist belongs to the original "normal" SPA page;
> 2. we finish the remaining work of stopping this CPU;
> 3. this CPU is completely stopped.
> 
> Between 1 and 3, this CPU can still receive interrupts (e.g. reschedule
> IPIs from CPU0, and Local APIC timer interrupts), and this CPU *must* write
> the EOI MSR for every interrupt received, otherwise the hypervisor may not
> deliver further interrupts, which may be needed to completely stop the CPU.
> 
> So, after we disable the EOI optimization in hv_cpu_die(), we need to make
> sure hvp->apic_assist's bit0 is zero. The easiest way is we just zero out
> the page when it's allocated in hv_cpu_init().
> 
> Note 1: after the "normal" SPA page is allocted and zeroed out, neither the
> hypervisor nor the guest writes into the page, so the page remains with
> zeros.
> 
> Note 2: see Section 10.3.5 "EOI Assist" for the details of the EOI
> optimization. When the optimization is enabled, the guest can still write
> the EOI MSR register irrespective of the "No EOI required" value, though
> by doing so we can't benefit from the optimization.
> 
> Fixes: ba696429d290 ("x86/hyper-v: Implement EOI assist")
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> 
> v2: there is no code change. I just improved the comment and the changelog
> according to the discussion with tglx:
> 
> https://lkml.org/lkml/2019/7/17/781
> https://lkml.org/lkml/2019/7/18/91 
> 
>  arch/x86/hyperv/hv_init.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 0e033ef11a9f..d26832cb38bb 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -60,8 +60,16 @@ static int hv_cpu_init(unsigned int cpu)
>  	if (!hv_vp_assist_page)
>  		return 0;
> 
> +	/*
> +	 * The VP ASSIST PAGE is an "overlay" page (see Hyper-V TLFS's Section
> +	 * 5.2.1 "GPA Overlay Pages"). Here it must be zeroed out to make sure
> +	 * we always write the EOI MSR in hv_apic_eoi_write() *after* the
> +	 * EOI optimization is disabled in hv_cpu_die(), otherwise a CPU may
> +	 * not be stopped in the case of CPU offlining and the VM will hang.
> +	 */
>  	if (!*hvp)
> -		*hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
> +		*hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL | __GFP_ZERO,
> +				 PAGE_KERNEL);
> 
>  	if (*hvp) {
>  		u64 val;
> --
> 2.19.1

Reviewed-by:  Michael Kelley <mikelley@microsoft.com>