* [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
@ 2012-02-21 11:30 Stefano Stabellini
2012-02-21 14:43 ` Konrad Rzeszutek Wilk
2012-03-04 16:06 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 8+ messages in thread
From: Stefano Stabellini @ 2012-02-21 11:30 UTC (permalink / raw)
To: konrad.wilk; +Cc: xen-devel, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
2012-02-21 11:30 [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND Stefano Stabellini
@ 2012-02-21 14:43 ` Konrad Rzeszutek Wilk
2012-03-04 16:06 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-02-21 14:43 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
2012-02-21 11:30 [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND Stefano Stabellini
2012-02-21 14:43 ` Konrad Rzeszutek Wilk
@ 2012-03-04 16:06 ` Konrad Rzeszutek Wilk
2012-03-05 22:45 ` Stefano Stabellini
1 sibling, 1 reply; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-04 16:06 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
2012-03-04 16:06 ` Konrad Rzeszutek Wilk
@ 2012-03-05 22:45 ` Stefano Stabellini
2012-03-13 16:29 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2012-03-05 22:45 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Stefano Stabellini
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
2012-03-05 22:45 ` Stefano Stabellini
@ 2012-03-13 16:29 ` Konrad Rzeszutek Wilk
2012-03-13 18:30 ` Stefano Stabellini
2012-03-13 23:34 ` Matt Wilson
0 siblings, 2 replies; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-13 16:29 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
2012-03-13 16:29 ` Konrad Rzeszutek Wilk
@ 2012-03-13 18:30 ` Stefano Stabellini
2012-03-13 23:20 ` Konrad Rzeszutek Wilk
2012-03-13 23:34 ` Matt Wilson
1 sibling, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2012-03-13 18:30 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Stefano Stabellini
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;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
2012-03-13 18:30 ` Stefano Stabellini
@ 2012-03-13 23:20 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-13 23:20 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel
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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND
2012-03-13 16:29 ` Konrad Rzeszutek Wilk
2012-03-13 18:30 ` Stefano Stabellini
@ 2012-03-13 23:34 ` Matt Wilson
1 sibling, 0 replies; 8+ messages in thread
From: Matt Wilson @ 2012-03-13 23:34 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-03-13 23:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-21 11:30 [PATCH] hvc_xen: introduce HVC_XEN_FRONTEND Stefano Stabellini
2012-02-21 14:43 ` Konrad Rzeszutek Wilk
2012-03-04 16:06 ` Konrad Rzeszutek Wilk
2012-03-05 22:45 ` Stefano Stabellini
2012-03-13 16:29 ` Konrad Rzeszutek Wilk
2012-03-13 18:30 ` Stefano Stabellini
2012-03-13 23:20 ` Konrad Rzeszutek Wilk
2012-03-13 23:34 ` Matt Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.