linux-kernel - RE: [PATCH 1/2] x86/hyperv: Fix hyperv_pcpu_input

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR21MB16889F38274F6F7691DB85F8D743A@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Mon, 22 May 2023 14:13:49 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
CC:     KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Dexuan Cui <decui@...rosoft.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "will@...nel.org" <will@...nel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>
Subject: RE: [PATCH 1/2] x86/hyperv: Fix hyperv_pcpu_input_arg handling when
 CPUs go online/offline

From: Vitaly Kuznetsov <vkuznets@...hat.com> Sent: Monday, May 22, 2023 1:56 AM
> 
> Michael Kelley <mikelley@...rosoft.com> writes:
> 

[snip]

> > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > index 0f1001d..696004a 100644
> > --- a/include/linux/cpuhotplug.h
> > +++ b/include/linux/cpuhotplug.h
> > @@ -201,6 +201,7 @@ enum cpuhp_state {
> >  	/* Online section invoked on the hotplugged CPU from the hotplug thread */
> >  	CPUHP_AP_ONLINE_IDLE,
> >  	CPUHP_AP_KVM_ONLINE,
> > +	CPUHP_AP_HYPERV_ONLINE,
> 
> (Cc: KVM)
> 
> I would very much prefer we swap the order with KVM here: hv_cpu_init()
> allocates and sets vCPU's VP assist page which is used by KVM on
> CPUHP_AP_KVM_ONLINE:
> 
> kvm_online_cpu() -> __hardware_enable_nolock() ->
> kvm_arch_hardware_enable() -> vmx_hardware_enable():
> 
>         /*
>          * This can happen if we hot-added a CPU but failed to allocate
>          * VP assist page for it.
>          */
> 	if (kvm_is_using_evmcs() && !hv_get_vp_assist_page(cpu))
> 		return -EFAULT;
> 
> With the change, this is likely broken.
> 
> FWIF, KVM also needs current vCPU's VP index (also set by hv_cpu_init())
> through __kvm_x86_vendor_init() -> set_hv_tscchange_cb() call chain but
> this happens upon KVM module load so CPU hotplug ordering should not
> matter as this always happens on a CPU which is already online.
> 
> Generally, as 'KVM on Hyper-V' is a supported scenario, doing Hyper-V
> init before KVM's (and vice versa on teardown) makes sense.
> 
> >  	CPUHP_AP_SCHED_WAIT_EMPTY,
> >  	CPUHP_AP_SMPBOOT_THREADS,
> >  	CPUHP_AP_X86_VDSO_VMA_ONLINE,

I have no objection to putting CPUHP_AP_HYPERV_ONLINE first.  I did
not give any consideration to a possible dependency between the two. :-(
But note that in current code, hv_cpu_init() is running on the
CPUHP_AP_ONLINE_DYN state, which is also after KVM.  So this patch
doesn't change the order w.r.t. KVM and the VP assist page.   Are things
already broken for KVM, or is something else happening that makes it
work anyway?

Michael