linux-kernel - Re: [Xen-devel] [PATCH] xen/xenbus: Add quirk to deal with misconfigured backends.

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] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.00.1203291210540.15151@kaball-desktop>
Date:	Thu, 29 Mar 2012 12:16:11 +0100
From:	Stefano Stabellini <stefano.stabellini@...citrix.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	Stefano Stabellini <Stefano.Stabellini@...citrix.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
	Ian Campbell <Ian.Campbell@...rix.com>
Subject: Re: [Xen-devel] [PATCH] xen/xenbus: Add quirk to deal with misconfigured
 backends.

On Wed, 28 Mar 2012, Konrad Rzeszutek Wilk wrote:
> Please take a look:
> 
> >From 85c9c09c69e3b4a277b2c930ca0a295f5bacc696 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> Date: Wed, 28 Mar 2012 14:55:45 -0400
> Subject: [PATCH] xen/xenbus: Add quirk to deal with misconfigured backends.
> 
> A rather annoying and common case is when booting a PVonHVM guest
> and exposing the PV KBD and PV VFB - as both of those do not
> make any sense. The HVM guest is using the VGA driver and the emulated
> keyboard for this (in Xen 4.2 and lower). We provide a very basic quirk
> framework to not wait for 6 minutes for those devices to initialize.
> 
> To trigger this, put this in your guest config:
> 
> vfb = [ 'vnc=1, vnclisten=0.0.0.0 ,vncunused=1']
> 
> instead of this:
> vnc=1
> vnclisten="0.0.0.0"
> 
> Note: The upstream QEMU can do PV KBD, so we can't just outright ignore
> the keyboard driver. Instead we change the timeout for that particular
> device to be much less.
> 
> [v1: Redo with timeout per Ian and Stefano suggestion]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> ---
>  drivers/xen/xenbus/xenbus_probe_frontend.c |   39 ++++++++++++++++++++++++++-
>  1 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
> index f20c5f1..09da9cf 100644
> --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> @@ -157,7 +157,40 @@ static int is_device_connecting(struct device *dev, void *data)
>  		(xendev->state == XenbusStateConnected &&
>  		 xendrv->is_ready && !xendrv->is_ready(xendev)));
>  }
> +static int check_for_quirks(struct device *dev, void *data)
> +{
> +	struct xenbus_device *xendev = to_xenbus_device(dev);
> +	struct device_driver *drv = data;
> +
> +	/* Is this operation limited to a particular driver? */
> +	if (drv && (dev->driver != drv))
> +		return false;
> +
> +	if (xen_pv_domain())
> +		return false;
>  
> +	/* With older QEMU, for PVonHVM guests the guest config files could
> +	 * contain: vfb = [ 'vnc=1, vnclisten=0.0.0.0 ,vncunused=1']
> +	 * which is nonsensical as there is no PV FB (or PV KBD) when
> +	 * running as HVM guest - at least with Xen 4.1 and Xen 4.2. */
> +
> +	if ((strncmp(xendev->nodename, "device/vkbd", 11) == 0))
> +		return true;
> +
> +	if ((strncmp(xendev->nodename, "device/vfb", 10) == 0))
> +		return true;
> +
> +	return false;
> +}
> +#define QUIRKS_TIMEOUT		30
> +#define DEFAULT_TIMEOUT		300
> +static unsigned int check_for_timeout_quirks(struct device_driver *drv, unsigned int timeout)
> +{
> +	if (bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
> +				check_for_quirks))
> +		return QUIRKS_TIMEOUT;
> +	return timeout;
> +}
>  static int exists_connecting_device(struct device_driver *drv)
>  {
>  	return bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
> @@ -211,18 +244,20 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
>  	unsigned long start = jiffies;
>  	struct device_driver *drv = xendrv ? &xendrv->driver : NULL;
>  	unsigned int seconds_waited = 0;
> +	unsigned int max_delay = DEFAULT_TIMEOUT;
>  
>  	if (!ready_to_wait_for_devices || !xen_domain())
>  		return;
>  
> +	max_delay = min(check_for_timeout_quirks(drv, max_delay), max_delay);
>  	while (exists_connecting_device(drv)) {
>  		if (time_after(jiffies, start + (seconds_waited+5)*HZ)) {
>  			if (!seconds_waited)
>  				printk(KERN_WARNING "XENBUS: Waiting for "
>  				       "devices to initialise: ");
>  			seconds_waited += 5;
> -			printk("%us...", 300 - seconds_waited);
> -			if (seconds_waited == 300)
> +			printk("%us...", max_delay - seconds_waited);
> +			if (seconds_waited == max_delay)
>  				break;
>  		}

This patch would reduce the timeout to 30 in case the guest is PV on HVM
and vkbd and/or vfb are present.
It is an improvement over the current situation but it doesn't directly
address the fact that critical PV devices, like PV disk and network,
could be slow and we actually need to wait for them.  Could we just have
two wait loops, one for critical devices and one for non-critical
devices?
First, with a timeout of 300, we would wait for disk and network, then
we would checkout if everything else is connected. If some other PV
devices, like VFB/VKBD, are not yet connected, we could decide whether
to continue anyway or wait for them some more.
--
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/