linux-kernel - Re: [PATCH] Drivers: hv: vmbus: prevent cpu offlining on newer hypervisors

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: <874mtl0y49.fsf@vitty.brq.redhat.com>
Date:	Thu, 27 Nov 2014 10:52:06 +0100
From:	Vitaly Kuznetsov <vkuznets@...hat.com>
To:	Dexuan Cui <decui@...rosoft.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"devel\@linuxdriverproject.org" <devel@...uxdriverproject.org>,
	"Haiyang Zhang" <haiyangz@...rosoft.com>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Drivers: hv: vmbus: prevent cpu offlining on newer hypervisors

Dexuan Cui <decui@...rosoft.com> writes:

>> -----Original Message-----
>> From: devel [mailto:driverdev-devel-bounces@...uxdriverproject.org] On
>> Behalf Of Greg Kroah-Hartman
>> Sent: Thursday, November 27, 2014 11:03 AM
>> To: Vitaly Kuznetsov
>> Cc: devel@...uxdriverproject.org; Haiyang Zhang; linux-
>> kernel@...r.kernel.org
>> Subject: Re: [PATCH] Drivers: hv: vmbus: prevent cpu offlining on newer
>> hypervisors
>> 
>> On Wed, Nov 26, 2014 at 02:52:22PM +0100, Vitaly Kuznetsov wrote:
>> > When an SMP Hyper-V guest is running on top of 2012R2 Server and
>> secondary
>> > cpus are sent offline (with echo 0 >
>> /sys/devices/system/cpu/cpu$cpu/online)
>> > the system freeze is observed. This happens due to the fact that on newer
>> > hypervisors (Win8, WS2012R2, ...) vmbus channel handlers are
>> distributed
>> > across all cpus (see init_vp_index() function in
>> drivers/hv/channel_mgmt.c)
>> > and on cpu offlining nobody reassigns them to CPU0. Prevent cpu
>> offlining
>> > when vmbus is loaded until the issue is fixed host-side.
>> >
>> > This patch also disables hibernation but it is OK as it is also broken (MCE
>> > error is hit on resume). Suspend still works.
>> >
>> > Tested with WS2008R2 and WS2012R2.
>> >
>> > Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
>> > ---
>> >  drivers/hv/vmbus_drv.c | 19 +++++++++++++++++++
>> >  1 file changed, 19 insertions(+)
>> >
>> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> > index 4d6b269..9a82249 100644
>> > --- a/drivers/hv/vmbus_drv.c
>> > +++ b/drivers/hv/vmbus_drv.c
>> > @@ -32,6 +32,7 @@
>> >  #include <linux/completion.h>
>> >  #include <linux/hyperv.h>
>> >  #include <linux/kernel_stat.h>
>> > +#include <linux/cpu.h>
>> >  #include <asm/hyperv.h>
>> >  #include <asm/hypervisor.h>
>> >  #include <asm/mshyperv.h>
>> > @@ -671,6 +672,13 @@ static void vmbus_isr(void)
>> >  		tasklet_schedule(&msg_dpc);
>> >  }
>> >
>> > +#ifdef CONFIG_HOTPLUG_CPU
>> > +static int hyperv_cpu_disable(void)
>> > +{
>> > +	return -1;
>> > +}
>> > +#endif
>> > +
>> >  /*
>> >   * vmbus_bus_init -Main vmbus driver initialization routine.
>> >   *
>> > @@ -711,6 +719,12 @@ static int vmbus_bus_init(int irq)
>> >  	if (ret)
>> >  		goto err_alloc;
>> >
>> > +#ifdef CONFIG_HOTPLUG_CPU
>> > +	if ((vmbus_proto_version != VERSION_WS2008) &&
>> > +	    (vmbus_proto_version != VERSION_WIN7))
>> > +		smp_ops.cpu_disable = hyperv_cpu_disable;
>> > +#endif
>> > +
>> >  	vmbus_request_offers();
>> >
>> >  	return 0;
>> > @@ -964,6 +978,11 @@ static void __exit vmbus_exit(void)
>> >  	bus_unregister(&hv_bus);
>> >  	hv_cleanup();
>> >  	acpi_bus_unregister_driver(&vmbus_acpi_driver);
>> > +#ifdef CONFIG_HOTPLUG_CPU
>> > +	if ((vmbus_proto_version != VERSION_WS2008) &&
>> > +	    (vmbus_proto_version != VERSION_WIN7))
>> > +		smp_ops.cpu_disable = native_cpu_disable;
>> > +#endif
>> >  }
>> 
>> #ifdef in a .c file is not a good idea to do if at all possible, please
>> only put this in one place, using a function call to "hide" the mess.
>> 
>> greg k-h
>
> Hi Vitaly,
> The idea of the patch is good to me.
>
> I agree with Greg.
> BTW, maybe hv_cpu_hotplug_quirk() is a better name?

My idea was that eventually this function will start doing something
real (e.g. switching channels to cpu0 if it doesn't happen fully
host-side) so I called it with a general name 'hyperv_cpu_disable'.

I'll try addressing our and Greg's comments in v2, thanks!

>
> Thanks,
> -- Dexuan

-- 
  Vitaly
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/