thr3ads.net - Xen devel - [PATCH] hvc_xen: introduce HVC_XEN

If this information is useful, please help other people find it:
Share via:

Stefano Stabellini

2012-Feb-21 11:30 UTC

[PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Introduce a new config option HVC_XEN_FRONTEND to enable/disable the
xenbus based pv console frontend.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/tty/hvc/Kconfig   |    8 +++
 drivers/tty/hvc/hvc_xen.c |  116 ++++++++++++++++++++++++---------------------
 2 files changed, 70 insertions(+), 54 deletions(-)

diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index 4222035..192e21e 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -76,6 +76,14 @@ config HVC_XEN
 	help
 	  Xen virtual console device driver
 
+config HVC_XEN_FRONTEND
+	bool "Xen Hypervisor Multiple Consoles support"
+	depends on HVC_XEN
+	select XEN_XENBUS_FRONTEND
+	default y
+	help
+	  Xen driver for secondary virtual consoles
+
 config HVC_UDBG
        bool "udbg based fake hypervisor console"
        depends on PPC && EXPERIMENTAL
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 26090c7..83d5c88 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -55,7 +55,6 @@ struct xencons_info {
 
 static LIST_HEAD(xenconsoles);
 static DEFINE_SPINLOCK(xencons_lock);
-static struct xenbus_driver xencons_driver;
 
 /* ------------------------------------------------------------------ */
 
@@ -298,53 +297,6 @@ static int xen_initial_domain_console_init(void)
 	return 0;
 }
 
-static int __init xen_hvc_init(void)
-{
-	int r;
-	struct xencons_info *info;
-	const struct hv_ops *ops;
-
-	if (!xen_domain())
-		return -ENODEV;
-
-	if (xen_initial_domain()) {
-		ops = &dom0_hvc_ops;
-		r = xen_initial_domain_console_init();
-		if (r < 0)
-			return r;
-		info = vtermno_to_xencons(HVC_COOKIE);
-	} else {
-		ops = &domU_hvc_ops;
-		if (xen_hvm_domain())
-			r = xen_hvm_console_init();
-		else
-			r = xen_pv_console_init();
-		if (r < 0)
-			return r;
-
-		info = vtermno_to_xencons(HVC_COOKIE);
-		info->irq = bind_evtchn_to_irq(info->evtchn);
-	}
-	if (info->irq < 0)
-		info->irq = 0; /* NO_IRQ */
-	else
-		irq_set_noprobe(info->irq);
-
-	info->hvc = hvc_alloc(HVC_COOKIE, info->irq, ops, 256);
-	if (IS_ERR(info->hvc)) {
-		r = PTR_ERR(info->hvc);
-		spin_lock(&xencons_lock);
-		list_del(&info->list);
-		spin_unlock(&xencons_lock);
-		if (info->irq)
-			unbind_from_irqhandler(info->irq, NULL);
-		kfree(info);
-		return r;
-	}
-
-	return xenbus_register_frontend(&xencons_driver);
-}
-
 void xen_console_resume(void)
 {
 	struct xencons_info *info = vtermno_to_xencons(HVC_COOKIE);
@@ -392,6 +344,9 @@ static int xen_console_remove(struct xencons_info *info)
 	return 0;
 }
 
+#ifdef CONFIG_HVC_XEN_FRONTEND
+static struct xenbus_driver xencons_driver;
+
 static int xencons_remove(struct xenbus_device *dev)
 {
 	return xen_console_remove(dev_get_drvdata(&dev->dev));
@@ -543,6 +498,65 @@ static const struct xenbus_device_id xencons_ids[] = {
 };
 
 
+static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
+	.probe = xencons_probe,
+	.remove = xencons_remove,
+	.resume = xencons_resume,
+	.otherend_changed = xencons_backend_changed,
+);
+#endif /* CONFIG_HVC_XEN_FRONTEND */
+
+static int __init xen_hvc_init(void)
+{
+	int r;
+	struct xencons_info *info;
+	const struct hv_ops *ops;
+
+	if (!xen_domain())
+		return -ENODEV;
+
+	if (xen_initial_domain()) {
+		ops = &dom0_hvc_ops;
+		r = xen_initial_domain_console_init();
+		if (r < 0)
+			return r;
+		info = vtermno_to_xencons(HVC_COOKIE);
+	} else {
+		ops = &domU_hvc_ops;
+		if (xen_hvm_domain())
+			r = xen_hvm_console_init();
+		else
+			r = xen_pv_console_init();
+		if (r < 0)
+			return r;
+
+		info = vtermno_to_xencons(HVC_COOKIE);
+		info->irq = bind_evtchn_to_irq(info->evtchn);
+	}
+	if (info->irq < 0)
+		info->irq = 0; /* NO_IRQ */
+	else
+		irq_set_noprobe(info->irq);
+
+	info->hvc = hvc_alloc(HVC_COOKIE, info->irq, ops, 256);
+	if (IS_ERR(info->hvc)) {
+		r = PTR_ERR(info->hvc);
+		spin_lock(&xencons_lock);
+		list_del(&info->list);
+		spin_unlock(&xencons_lock);
+		if (info->irq)
+			unbind_from_irqhandler(info->irq, NULL);
+		kfree(info);
+		return r;
+	}
+
+	r = 0;
+#ifdef CONFIG_HVC_XEN_FRONTEND
+	r = xenbus_register_frontend(&xencons_driver);
+#endif
+	return r;
+}
+
 static void __exit xen_hvc_fini(void)
 {
 	struct xencons_info *entry, *next;
@@ -580,12 +594,6 @@ static int xen_cons_init(void)
 	return 0;
 }
 
-static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
-	.probe = xencons_probe,
-	.remove = xencons_remove,
-	.resume = xencons_resume,
-	.otherend_changed = xencons_backend_changed,
-);
 
 module_init(xen_hvc_init);
 module_exit(xen_hvc_fini);
-- 
1.7.2.5

Konrad Rzeszutek Wilk

2012-Feb-21 14:43 UTC

head link

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

On Tue, Feb 21, 2012 at 11:30:42AM +0000, Stefano Stabellini
wrote:> Introduce a new config option HVC_XEN_FRONTEND to enable/disable the
> xenbus based pv console frontend.
applied.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  drivers/tty/hvc/Kconfig   |    8 +++
>  drivers/tty/hvc/hvc_xen.c |  116
++++++++++++++++++++++++---------------------
>  2 files changed, 70 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
> index 4222035..192e21e 100644
> --- a/drivers/tty/hvc/Kconfig
> +++ b/drivers/tty/hvc/Kconfig
> @@ -76,6 +76,14 @@ config HVC_XEN
>  	help
>  	  Xen virtual console device driver
>  
> +config HVC_XEN_FRONTEND
> +	bool "Xen Hypervisor Multiple Consoles support"
> +	depends on HVC_XEN
> +	select XEN_XENBUS_FRONTEND
> +	default y
> +	help
> +	  Xen driver for secondary virtual consoles
> +
>  config HVC_UDBG
>         bool "udbg based fake hypervisor console"
>         depends on PPC && EXPERIMENTAL
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 26090c7..83d5c88 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -55,7 +55,6 @@ struct xencons_info {
>  
>  static LIST_HEAD(xenconsoles);
>  static DEFINE_SPINLOCK(xencons_lock);
> -static struct xenbus_driver xencons_driver;
>  
>  /* ------------------------------------------------------------------ */
>  
> @@ -298,53 +297,6 @@ static int xen_initial_domain_console_init(void)
>  	return 0;
>  }
>  
> -static int __init xen_hvc_init(void)
> -{
> -	int r;
> -	struct xencons_info *info;
> -	const struct hv_ops *ops;
> -
> -	if (!xen_domain())
> -		return -ENODEV;
> -
> -	if (xen_initial_domain()) {
> -		ops = &dom0_hvc_ops;
> -		r = xen_initial_domain_console_init();
> -		if (r < 0)
> -			return r;
> -		info = vtermno_to_xencons(HVC_COOKIE);
> -	} else {
> -		ops = &domU_hvc_ops;
> -		if (xen_hvm_domain())
> -			r = xen_hvm_console_init();
> -		else
> -			r = xen_pv_console_init();
> -		if (r < 0)
> -			return r;
> -
> -		info = vtermno_to_xencons(HVC_COOKIE);
> -		info->irq = bind_evtchn_to_irq(info->evtchn);
> -	}
> -	if (info->irq < 0)
> -		info->irq = 0; /* NO_IRQ */
> -	else
> -		irq_set_noprobe(info->irq);
> -
> -	info->hvc = hvc_alloc(HVC_COOKIE, info->irq, ops, 256);
> -	if (IS_ERR(info->hvc)) {
> -		r = PTR_ERR(info->hvc);
> -		spin_lock(&xencons_lock);
> -		list_del(&info->list);
> -		spin_unlock(&xencons_lock);
> -		if (info->irq)
> -			unbind_from_irqhandler(info->irq, NULL);
> -		kfree(info);
> -		return r;
> -	}
> -
> -	return xenbus_register_frontend(&xencons_driver);
> -}
> -
>  void xen_console_resume(void)
>  {
>  	struct xencons_info *info = vtermno_to_xencons(HVC_COOKIE);
> @@ -392,6 +344,9 @@ static int xen_console_remove(struct xencons_info
*info)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_HVC_XEN_FRONTEND
> +static struct xenbus_driver xencons_driver;
> +
>  static int xencons_remove(struct xenbus_device *dev)
>  {
>  	return xen_console_remove(dev_get_drvdata(&dev->dev));
> @@ -543,6 +498,65 @@ static const struct xenbus_device_id xencons_ids[] = {
>  };
>  
>  
> +static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
> +	.probe = xencons_probe,
> +	.remove = xencons_remove,
> +	.resume = xencons_resume,
> +	.otherend_changed = xencons_backend_changed,
> +);
> +#endif /* CONFIG_HVC_XEN_FRONTEND */
> +
> +static int __init xen_hvc_init(void)
> +{
> +	int r;
> +	struct xencons_info *info;
> +	const struct hv_ops *ops;
> +
> +	if (!xen_domain())
> +		return -ENODEV;
> +
> +	if (xen_initial_domain()) {
> +		ops = &dom0_hvc_ops;
> +		r = xen_initial_domain_console_init();
> +		if (r < 0)
> +			return r;
> +		info = vtermno_to_xencons(HVC_COOKIE);
> +	} else {
> +		ops = &domU_hvc_ops;
> +		if (xen_hvm_domain())
> +			r = xen_hvm_console_init();
> +		else
> +			r = xen_pv_console_init();
> +		if (r < 0)
> +			return r;
> +
> +		info = vtermno_to_xencons(HVC_COOKIE);
> +		info->irq = bind_evtchn_to_irq(info->evtchn);
> +	}
> +	if (info->irq < 0)
> +		info->irq = 0; /* NO_IRQ */
> +	else
> +		irq_set_noprobe(info->irq);
> +
> +	info->hvc = hvc_alloc(HVC_COOKIE, info->irq, ops, 256);
> +	if (IS_ERR(info->hvc)) {
> +		r = PTR_ERR(info->hvc);
> +		spin_lock(&xencons_lock);
> +		list_del(&info->list);
> +		spin_unlock(&xencons_lock);
> +		if (info->irq)
> +			unbind_from_irqhandler(info->irq, NULL);
> +		kfree(info);
> +		return r;
> +	}
> +
> +	r = 0;
> +#ifdef CONFIG_HVC_XEN_FRONTEND
> +	r = xenbus_register_frontend(&xencons_driver);
> +#endif
> +	return r;
> +}
> +
>  static void __exit xen_hvc_fini(void)
>  {
>  	struct xencons_info *entry, *next;
> @@ -580,12 +594,6 @@ static int xen_cons_init(void)
>  	return 0;
>  }
>  
> -static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
> -	.probe = xencons_probe,
> -	.remove = xencons_remove,
> -	.resume = xencons_resume,
> -	.otherend_changed = xencons_backend_changed,
> -);
>  
>  module_init(xen_hvc_init);
>  module_exit(xen_hvc_fini);
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xensource.com/xen-devel

Konrad Rzeszutek Wilk

2012-Mar-04 16:06 UTC

head link

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

On Tue, Feb 21, 2012 at 11:30:42AM +0000, Stefano Stabellini
wrote:> Introduce a new config option HVC_XEN_FRONTEND to enable/disable the
> xenbus based pv console frontend.
One concern I have - not with this patch - but rather with the whole
PV consoel functionality - is how it is going to work with older
hypervisors/QEMU.

I am specifically thinking about Amazon EC2 or 3.4 Xen installations.
I recall that a patch was required to QEMU to make this work flawlessly - so
perhaps all of this code should be gated on checking fora version (so Xen 4.2?)
or by looking for a ''feature-pv-on-hvm-console'' XenBus
attribute?

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  drivers/tty/hvc/Kconfig   |    8 +++
>  drivers/tty/hvc/hvc_xen.c |  116
++++++++++++++++++++++++---------------------
>  2 files changed, 70 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
> index 4222035..192e21e 100644
> --- a/drivers/tty/hvc/Kconfig
> +++ b/drivers/tty/hvc/Kconfig
> @@ -76,6 +76,14 @@ config HVC_XEN
>  	help
>  	  Xen virtual console device driver
>  
> +config HVC_XEN_FRONTEND
> +	bool "Xen Hypervisor Multiple Consoles support"
> +	depends on HVC_XEN
> +	select XEN_XENBUS_FRONTEND
> +	default y
> +	help
> +	  Xen driver for secondary virtual consoles
> +
>  config HVC_UDBG
>         bool "udbg based fake hypervisor console"
>         depends on PPC && EXPERIMENTAL
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 26090c7..83d5c88 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -55,7 +55,6 @@ struct xencons_info {
>  
>  static LIST_HEAD(xenconsoles);
>  static DEFINE_SPINLOCK(xencons_lock);
> -static struct xenbus_driver xencons_driver;
>  
>  /* ------------------------------------------------------------------ */
>  
> @@ -298,53 +297,6 @@ static int xen_initial_domain_console_init(void)
>  	return 0;
>  }
>  
> -static int __init xen_hvc_init(void)
> -{
> -	int r;
> -	struct xencons_info *info;
> -	const struct hv_ops *ops;
> -
> -	if (!xen_domain())
> -		return -ENODEV;
> -
> -	if (xen_initial_domain()) {
> -		ops = &dom0_hvc_ops;
> -		r = xen_initial_domain_console_init();
> -		if (r < 0)
> -			return r;
> -		info = vtermno_to_xencons(HVC_COOKIE);
> -	} else {
> -		ops = &domU_hvc_ops;
> -		if (xen_hvm_domain())
> -			r = xen_hvm_console_init();
> -		else
> -			r = xen_pv_console_init();
> -		if (r < 0)
> -			return r;
> -
> -		info = vtermno_to_xencons(HVC_COOKIE);
> -		info->irq = bind_evtchn_to_irq(info->evtchn);
> -	}
> -	if (info->irq < 0)
> -		info->irq = 0; /* NO_IRQ */
> -	else
> -		irq_set_noprobe(info->irq);
> -
> -	info->hvc = hvc_alloc(HVC_COOKIE, info->irq, ops, 256);
> -	if (IS_ERR(info->hvc)) {
> -		r = PTR_ERR(info->hvc);
> -		spin_lock(&xencons_lock);
> -		list_del(&info->list);
> -		spin_unlock(&xencons_lock);
> -		if (info->irq)
> -			unbind_from_irqhandler(info->irq, NULL);
> -		kfree(info);
> -		return r;
> -	}
> -
> -	return xenbus_register_frontend(&xencons_driver);
> -}
> -
>  void xen_console_resume(void)
>  {
>  	struct xencons_info *info = vtermno_to_xencons(HVC_COOKIE);
> @@ -392,6 +344,9 @@ static int xen_console_remove(struct xencons_info
*info)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_HVC_XEN_FRONTEND
> +static struct xenbus_driver xencons_driver;
> +
>  static int xencons_remove(struct xenbus_device *dev)
>  {
>  	return xen_console_remove(dev_get_drvdata(&dev->dev));
> @@ -543,6 +498,65 @@ static const struct xenbus_device_id xencons_ids[] = {
>  };
>  
>  
> +static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
> +	.probe = xencons_probe,
> +	.remove = xencons_remove,
> +	.resume = xencons_resume,
> +	.otherend_changed = xencons_backend_changed,
> +);
> +#endif /* CONFIG_HVC_XEN_FRONTEND */
> +
> +static int __init xen_hvc_init(void)
> +{
> +	int r;
> +	struct xencons_info *info;
> +	const struct hv_ops *ops;
> +
> +	if (!xen_domain())
> +		return -ENODEV;
> +
> +	if (xen_initial_domain()) {
> +		ops = &dom0_hvc_ops;
> +		r = xen_initial_domain_console_init();
> +		if (r < 0)
> +			return r;
> +		info = vtermno_to_xencons(HVC_COOKIE);
> +	} else {
> +		ops = &domU_hvc_ops;
> +		if (xen_hvm_domain())
> +			r = xen_hvm_console_init();
> +		else
> +			r = xen_pv_console_init();
> +		if (r < 0)
> +			return r;
> +
> +		info = vtermno_to_xencons(HVC_COOKIE);
> +		info->irq = bind_evtchn_to_irq(info->evtchn);
> +	}
> +	if (info->irq < 0)
> +		info->irq = 0; /* NO_IRQ */
> +	else
> +		irq_set_noprobe(info->irq);
> +
> +	info->hvc = hvc_alloc(HVC_COOKIE, info->irq, ops, 256);
> +	if (IS_ERR(info->hvc)) {
> +		r = PTR_ERR(info->hvc);
> +		spin_lock(&xencons_lock);
> +		list_del(&info->list);
> +		spin_unlock(&xencons_lock);
> +		if (info->irq)
> +			unbind_from_irqhandler(info->irq, NULL);
> +		kfree(info);
> +		return r;
> +	}
> +
> +	r = 0;
> +#ifdef CONFIG_HVC_XEN_FRONTEND
> +	r = xenbus_register_frontend(&xencons_driver);
> +#endif
> +	return r;
> +}
> +
>  static void __exit xen_hvc_fini(void)
>  {
>  	struct xencons_info *entry, *next;
> @@ -580,12 +594,6 @@ static int xen_cons_init(void)
>  	return 0;
>  }
>  
> -static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
> -	.probe = xencons_probe,
> -	.remove = xencons_remove,
> -	.resume = xencons_resume,
> -	.otherend_changed = xencons_backend_changed,
> -);
>  
>  module_init(xen_hvc_init);
>  module_exit(xen_hvc_fini);
> -- 
> 1.7.2.5

Stefano Stabellini

2012-Mar-05 22:45 UTC

head link

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

On Sun, 4 Mar 2012, Konrad Rzeszutek Wilk wrote:> On Tue, Feb 21, 2012 at 11:30:42AM +0000, Stefano Stabellini wrote:
> > Introduce a new config option HVC_XEN_FRONTEND to enable/disable the
> > xenbus based pv console frontend.
> 
> One concern I have - not with this patch - but rather with the whole
> PV consoel functionality - is how it is going to work with older
hypervisors/QEMU.
> 
> I am specifically thinking about Amazon EC2 or 3.4 Xen installations.
> I recall that a patch was required to QEMU to make this work flawlessly -
so
> perhaps all of this code should be gated on checking fora version (so Xen
4.2?)
> or by looking for a ''feature-pv-on-hvm-console'' XenBus
attribute?
I agree that this issue needs more thoughts about compatibility with old
xen installations, but if it is possible I would rather avoid
introducing a this-bug-is-now-fixed kind of flag.

Only xen installations supporting vfb and qemu as a console backend are
susceptible, so Amazon should be safe because I don''t think they use
any
of them.
Also looking through the code I have found that qemu-xen 3.4, 4.0 and
4.1 are all susceptible to this bug the same way and they can all be
fixed with the same patch.

From the Linux side the best thing we could do is completely ignore
devices/console/0, the problem is that if we don''t we are either going
to upset QEMU or xenconsoled.
If we return 0 from xencons_probe we are going to pretend everything is
normal so as a consequence the xenbus state is going to be set to 4,
upsetting xenconsoled.
If we return -ENODEV we are going to admit that the device shouldn''t
even be there and in that case the xenbus drivers set the state to 6
causing the unpatched qemu to crash.
I don''t think there is anything we can do within xencons_probe to avoid
the bug: what return value are we supposed to return so that the
xenbus drivers does not take any further actions?
Even a ''feature-pv-on-hvm-console'' flag  wouldn''t
help.

Maybe we need to introduce an explicit check in xenbus_probe_device_type
to avoid calling bus->probe if type == "console" and dir[i] ==
"0", what
do you think?

Konrad Rzeszutek Wilk

2012-Mar-13 16:29 UTC

head link

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

On Mon, Mar 05, 2012 at 10:45:33PM +0000, Stefano Stabellini
wrote:> On Sun, 4 Mar 2012, Konrad Rzeszutek Wilk wrote:
> > On Tue, Feb 21, 2012 at 11:30:42AM +0000, Stefano Stabellini wrote:
> > > Introduce a new config option HVC_XEN_FRONTEND to enable/disable
the
> > > xenbus based pv console frontend.
> > 
> > One concern I have - not with this patch - but rather with the whole
> > PV consoel functionality - is how it is going to work with older
hypervisors/QEMU.
> > 
> > I am specifically thinking about Amazon EC2 or 3.4 Xen installations.
> > I recall that a patch was required to QEMU to make this work
flawlessly - so
> > perhaps all of this code should be gated on checking fora version (so
Xen 4.2?)
> > or by looking for a ''feature-pv-on-hvm-console''
XenBus attribute?
> 
> I agree that this issue needs more thoughts about compatibility with old
> xen installations, but if it is possible I would rather avoid
> introducing a this-bug-is-now-fixed kind of flag.
Think of it as working around broken implementations. Like dealing with BIOSes
that aren''t exactly working right.
> 
> Only xen installations supporting vfb and qemu as a console backend are
> susceptible, so Amazon should be safe because I don''t think they
use any
> of them.
I think they use the normal xenconsole .. but then the patch to return 0
would work .. but upset future version of QEMU (or is it the other way
around).
> Also looking through the code I have found that qemu-xen 3.4, 4.0 and
> 4.1 are all susceptible to this bug the same way and they can all be
> fixed with the same patch.
> 
> >From the Linux side the best thing we could do is completely ignore
> devices/console/0, the problem is that if we don''t we are either
going
> to upset QEMU or xenconsoled.
> If we return 0 from xencons_probe we are going to pretend everything is
> normal so as a consequence the xenbus state is going to be set to 4,
> upsetting xenconsoled.
> If we return -ENODEV we are going to admit that the device
shouldn''t
> even be there and in that case the xenbus drivers set the state to 6
> causing the unpatched qemu to crash.
> I don''t think there is anything we can do within xencons_probe to
avoid
> the bug: what return value are we supposed to return so that the
> xenbus drivers does not take any further actions?
Right. So I was thinking that finding out what hypervisor is present - 
if 4.2, then it is OK to assume QEMU has it fixed.
> Even a ''feature-pv-on-hvm-console'' flag 
wouldn''t help.
> 
> Maybe we need to introduce an explicit check in xenbus_probe_device_type
> to avoid calling bus->probe if type == "console" and dir[i] ==
"0", what
> do you think?
If that works..?
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Stefano Stabellini

2012-Mar-13 18:30 UTC

head link

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

On Tue, 13 Mar 2012, Konrad Rzeszutek Wilk wrote:> > Even a ''feature-pv-on-hvm-console'' flag 
wouldn''t help.
> > 
> > Maybe we need to introduce an explicit check in
xenbus_probe_device_type
> > to avoid calling bus->probe if type == "console" and
dir[i] == "0", what
> > do you think?
> 
> If that works..?
Yes, it does: the appended patch might be ugly but fixes the problem for
me (tested xen 4.1, xm/xend, vfb enabled and disabled so both qemu and
xenconsoled as console backends).

---

xenbus: ignore console/0

Unfortunately xend creates a bogus console/0 frotend/backend entry pair
on xenstore that console backends cannot properly cope with.
Any guest behavior that is not completely ignoring console/0 is going
to either cause problems with xenconsoled or qemu.
Returning 0 or -ENODEV from xencons_probe is not enough because it is
going to cause the frontend state to become 4 or 6 respectively.
The best possible thing we can do here is just ignore the entry from
xenbus_probe_frontend.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c
b/drivers/xen/xenbus/xenbus_probe_frontend.c
index 9c57819..f20c5f1 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -53,6 +53,12 @@ static int xenbus_probe_frontend(struct xen_bus_type *bus,
const char *type,
 	char *nodename;
 	int err;
 
+	/* ignore console/0 */
+	if (!strncmp(type, "console", 7) && !strncmp(name,
"0", 1)) {
+		DPRINTK("Ignoring buggy device entry console/0");
+		return 0;
+	}
+
 	nodename = kasprintf(GFP_KERNEL, "%s/%s/%s", bus->root, type,
name);
 	if (!nodename)
 		return -ENOMEM;

Konrad Rzeszutek Wilk

2012-Mar-13 23:20 UTC

head link

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

On Tue, Mar 13, 2012 at 06:30:44PM +0000, Stefano Stabellini
wrote:> On Tue, 13 Mar 2012, Konrad Rzeszutek Wilk wrote:
> > > Even a ''feature-pv-on-hvm-console'' flag 
wouldn''t help.
> > > 
> > > Maybe we need to introduce an explicit check in
xenbus_probe_device_type
> > > to avoid calling bus->probe if type == "console" and
dir[i] == "0", what
> > > do you think?
> > 
> > If that works..?
> 
> Yes, it does: the appended patch might be ugly but fixes the problem for
> me (tested xen 4.1, xm/xend, vfb enabled and disabled so both qemu and
> xenconsoled as console backends).
Yup. Seems to work with PV and HVM in both Xen 4.1 (with patch) and Xen 4.0.
> 
> ---
> 
> xenbus: ignore console/0
> 
> Unfortunately xend creates a bogus console/0 frotend/backend entry pair
> on xenstore that console backends cannot properly cope with.
> Any guest behavior that is not completely ignoring console/0 is going
> to either cause problems with xenconsoled or qemu.
> Returning 0 or -ENODEV from xencons_probe is not enough because it is
> going to cause the frontend state to become 4 or 6 respectively.
> The best possible thing we can do here is just ignore the entry from
> xenbus_probe_frontend.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c
b/drivers/xen/xenbus/xenbus_probe_frontend.c
> index 9c57819..f20c5f1 100644
> --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> @@ -53,6 +53,12 @@ static int xenbus_probe_frontend(struct xen_bus_type
*bus, const char *type,
>  	char *nodename;
>  	int err;
>  
> +	/* ignore console/0 */
> +	if (!strncmp(type, "console", 7) && !strncmp(name,
"0", 1)) {
> +		DPRINTK("Ignoring buggy device entry console/0");
> +		return 0;
> +	}
> +
>  	nodename = kasprintf(GFP_KERNEL, "%s/%s/%s", bus->root,
type, name);
>  	if (!nodename)
>  		return -ENOMEM;

Matt Wilson

2012-Mar-13 23:34 UTC

head link

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

On Tue, Mar 13, 2012 at 09:29:44AM -0700, Konrad Rzeszutek Wilk
wrote:> On Mon, Mar 05, 2012 at 10:45:33PM +0000, Stefano Stabellini wrote:
> > Only xen installations supporting vfb and qemu as a console backend
are
> > susceptible, so Amazon should be safe because I don''t think
they use any
> > of them.
> 
> I think they use the normal xenconsole .. but then the patch to return 0
> would work .. but upset future version of QEMU (or is it the other way
> around).
There are HVM instance types in EC2: Cluster Compute (HPC cc1.*, cg1.*
and cc2.* instances) and all instances running Windows. When booting
Linux on a Cluster Compute instance you may see:

[    1.887398] XENBUS: Device with no driver: device/vfb/0
[    1.890850] XENBUS: Device with no driver: device/console/0

It''s not safe to assume anything in regard to breaking compatibility
with older versions of Xen or QEMU.

Matt

Xen devel - Feb 2012 - [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

[PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND

Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND