[PATCH 0/2] Xen (PV and HVM) multiple PV consoles

[PATCH 0/2] Xen (PV and HVM) multiple PV consoles

[Stefano Stabellini]

Hi all,
this small patch series (sent before a few times) achieves two goals:

- make PV consoles work for PV on HVM guests;

- implement support for multiple PV consoles for PV and PV on HVM guests.


It has been tested with pv and hvm guests, with console=ttyS0,
console=hvc0, and earlyprintk=xen, with and without serial='pty' in the
VM config file.


Stefano Stabellini (2):
      hvc_xen: support PV on HVM consoles
      hvc_xen: implement multiconsole support

 drivers/tty/hvc/hvc_xen.c          |  461 ++++++++++++++++++++++++++++++++---
 include/xen/interface/hvm/params.h |    6 +-
 2 files changed, 426 insertions(+), 41 deletions(-)

A branch with these patches based on 3.2 is available here:

git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git 3.2-multiconsole

Cheers,

Stefano

[PATCH 1/2] hvc_xen: support PV on HVM consoles

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/tty/hvc/hvc_xen.c          |   86 +++++++++++++++++++++++++++++-------
 include/xen/interface/hvm/params.h |    6 ++-
 2 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 52fdf60..dd6641f 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -24,9 +24,12 @@
 #include <linux/init.h>
 #include <linux/types.h>
 
+#include <asm/io.h>
 #include <asm/xen/hypervisor.h>
 
 #include <xen/xen.h>
+#include <xen/interface/xen.h>
+#include <xen/hvm.h>
 #include <xen/page.h>
 #include <xen/events.h>
 #include <xen/interface/io/console.h>
@@ -42,9 +45,13 @@ static int xencons_irq;
 /* ------------------------------------------------------------------ */
 
 static unsigned long console_pfn = ~0ul;
+static unsigned int console_evtchn = ~0ul;
+static struct xencons_interface *xencons_if = NULL;
 
 static inline struct xencons_interface *xencons_interface(void)
 {
+	if (xencons_if != NULL)
+		return xencons_if;
 	if (console_pfn == ~0ul)
 		return mfn_to_virt(xen_start_info->console.domU.mfn);
 	else
@@ -54,7 +61,10 @@ static inline struct xencons_interface *xencons_interface(void)
 static inline void notify_daemon(void)
 {
 	/* Use evtchn: this is called early, before irq is set up. */
-	notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
+	if (console_evtchn == ~0ul)
+		notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
+	else
+		notify_remote_via_evtchn(console_evtchn);
 }
 
 static int __write_console(const char *data, int len)
@@ -157,28 +167,65 @@ static struct hv_ops dom0_hvc_ops = {
 	.notifier_hangup = notifier_hangup_irq,
 };
 
+static int xen_hvm_console_init(void)
+{
+	int r;
+	uint64_t v = 0;
+	unsigned long mfn;
+
+	if (!xen_hvm_domain())
+		return -ENODEV;
+
+	if (xencons_if != NULL)
+		return -EBUSY;
+
+	r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
+	if (r < 0)
+		return -ENODEV;
+	console_evtchn = v;
+	hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
+	if (r < 0)
+		return -ENODEV;
+	mfn = v;
+	xencons_if = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
+	if (xencons_if == NULL)
+		return -ENODEV;
+
+	return 0;
+}
+
 static int __init xen_hvc_init(void)
 {
 	struct hvc_struct *hp;
 	struct hv_ops *ops;
+	int r;
 
-	if (!xen_pv_domain())
+	if (!xen_domain())
+		return -ENODEV;
+
+	if (xen_pv_domain() && !xen_initial_domain() &&
+			!xen_start_info->console.domU.evtchn)
 		return -ENODEV;
 
 	if (xen_initial_domain()) {
 		ops = &dom0_hvc_ops;
 		xencons_irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
 	} else {
-		if (!xen_start_info->console.domU.evtchn)
-			return -ENODEV;
-
 		ops = &domU_hvc_ops;
-		xencons_irq = bind_evtchn_to_irq(xen_start_info->console.domU.evtchn);
+		if (xen_pv_domain()) {
+			console_pfn = mfn_to_pfn(xen_start_info->console.domU.mfn);
+			console_evtchn = xen_start_info->console.domU.evtchn;
+		} else {
+			r = xen_hvm_console_init();
+			if (r < 0)
+				return r;
+		}
+		xencons_irq = bind_evtchn_to_irq(console_evtchn);
+		if (xencons_irq < 0)
+			xencons_irq = 0; /* NO_IRQ */
+		else
+			irq_set_noprobe(xencons_irq);
 	}
-	if (xencons_irq < 0)
-		xencons_irq = 0; /* NO_IRQ */
-	else
-		irq_set_noprobe(xencons_irq);
 
 	hp = hvc_alloc(HVC_COOKIE, xencons_irq, ops, 256);
 	if (IS_ERR(hp))
@@ -186,15 +233,13 @@ static int __init xen_hvc_init(void)
 
 	hvc = hp;
 
-	console_pfn = mfn_to_pfn(xen_start_info->console.domU.mfn);
-
 	return 0;
 }
 
 void xen_console_resume(void)
 {
 	if (xencons_irq)
-		rebind_evtchn_irq(xen_start_info->console.domU.evtchn, xencons_irq);
+		rebind_evtchn_irq(console_evtchn, xencons_irq);
 }
 
 static void __exit xen_hvc_fini(void)
@@ -205,16 +250,22 @@ static void __exit xen_hvc_fini(void)
 
 static int xen_cons_init(void)
 {
-	struct hv_ops *ops;
+	const struct hv_ops *ops;
 
-	if (!xen_pv_domain())
+	if (!xen_domain())
 		return 0;
 
 	if (xen_initial_domain())
 		ops = &dom0_hvc_ops;
-	else
+	else {
 		ops = &domU_hvc_ops;
 
+		if (xen_pv_domain())
+			console_evtchn = xen_start_info->console.domU.evtchn;
+		else
+			xen_hvm_console_init();
+	}
+
 	hvc_instantiate(HVC_COOKIE, 0, ops);
 	return 0;
 }
@@ -230,6 +281,9 @@ static void xenboot_write_console(struct console *console, const char *string,
 	unsigned int linelen, off = 0;
 	const char *pos;
 
+	if (!xen_pv_domain())
+		return;
+
 	dom0_write_console(0, string, len);
 
 	if (xen_initial_domain())
diff --git a/include/xen/interface/hvm/params.h b/include/xen/interface/hvm/params.h
index 1888d8c..1b4f923 100644
--- a/include/xen/interface/hvm/params.h
+++ b/include/xen/interface/hvm/params.h
@@ -90,6 +90,10 @@
 /* Boolean: Enable aligning all periodic vpts to reduce interrupts */
 #define HVM_PARAM_VPT_ALIGN    16
 
-#define HVM_NR_PARAMS          17
+/* Console debug shared memory ring and event channel */
+#define HVM_PARAM_CONSOLE_PFN    17
+#define HVM_PARAM_CONSOLE_EVTCHN 18
+
+#define HVM_NR_PARAMS          19
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
-- 
1.7.2.5

[PATCH 2/2] hvc_xen: implement multiconsole support

[Stefano Stabellini]

This patch implements support for multiple consoles:
consoles other than the first one are setup using the traditional xenbus
and grant-table based mechanism.
We use a list to keep track of the allocated consoles, we don't
expect too many of them anyway.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/tty/hvc/hvc_xen.c |  439 +++++++++++++++++++++++++++++++++++++++------
 1 files changed, 383 insertions(+), 56 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index dd6641f..97732fb 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -23,6 +23,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/types.h>
+#include <linux/list.h>
 
 #include <asm/io.h>
 #include <asm/xen/hypervisor.h>
@@ -30,47 +31,69 @@
 #include <xen/xen.h>
 #include <xen/interface/xen.h>
 #include <xen/hvm.h>
+#include <xen/grant_table.h>
 #include <xen/page.h>
 #include <xen/events.h>
 #include <xen/interface/io/console.h>
 #include <xen/hvc-console.h>
+#include <xen/xenbus.h>
 
 #include "hvc_console.h"
 
 #define HVC_COOKIE   0x58656e /* "Xen" in hex */
 
-static struct hvc_struct *hvc;
-static int xencons_irq;
+struct xencons_info {
+	struct list_head list;
+	struct xenbus_device *xbdev;
+	struct xencons_interface *intf;
+	unsigned int evtchn;
+	struct hvc_struct *hvc;
+	int irq;
+	int vtermno;
+	grant_ref_t gntref;
+};
+
+static LIST_HEAD(xenconsoles);
+static DEFINE_SPINLOCK(xencons_lock);
+static struct xenbus_driver xencons_driver;
 
 /* ------------------------------------------------------------------ */
 
-static unsigned long console_pfn = ~0ul;
-static unsigned int console_evtchn = ~0ul;
-static struct xencons_interface *xencons_if = NULL;
+static struct xencons_info *vtermno_to_xencons(int vtermno)
+{
+	struct xencons_info *entry, *ret = NULL;
+
+	if (list_empty(&xenconsoles))
+			return NULL;
 
-static inline struct xencons_interface *xencons_interface(void)
+	spin_lock(&xencons_lock);
+	list_for_each_entry(entry, &xenconsoles, list) {
+		if (entry->vtermno == vtermno) {
+			ret  = entry;
+			break;
+		}
+	}
+	spin_unlock(&xencons_lock);
+
+	return ret;
+}
+
+static inline int xenbus_devid_to_vtermno(int devid)
 {
-	if (xencons_if != NULL)
-		return xencons_if;
-	if (console_pfn == ~0ul)
-		return mfn_to_virt(xen_start_info->console.domU.mfn);
-	else
-		return __va(console_pfn << PAGE_SHIFT);
+	return devid + HVC_COOKIE;
 }
 
-static inline void notify_daemon(void)
+static inline void notify_daemon(struct xencons_info *cons)
 {
 	/* Use evtchn: this is called early, before irq is set up. */
-	if (console_evtchn == ~0ul)
-		notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
-	else
-		notify_remote_via_evtchn(console_evtchn);
+	notify_remote_via_evtchn(cons->evtchn);
 }
 
-static int __write_console(const char *data, int len)
+static int __write_console(struct xencons_info *xencons,
+		const char *data, int len)
 {
-	struct xencons_interface *intf = xencons_interface();
 	XENCONS_RING_IDX cons, prod;
+	struct xencons_interface *intf = xencons->intf;
 	int sent = 0;
 
 	cons = intf->out_cons;
@@ -85,13 +108,16 @@ static int __write_console(const char *data, int len)
 	intf->out_prod = prod;
 
 	if (sent)
-		notify_daemon();
+		notify_daemon(xencons);
 	return sent;
 }
 
 static int domU_write_console(uint32_t vtermno, const char *data, int len)
 {
 	int ret = len;
+	struct xencons_info *cons = vtermno_to_xencons(vtermno);
+	if (cons == NULL)
+		return -EINVAL;
 
 	/*
 	 * Make sure the whole buffer is emitted, polling if
@@ -100,7 +126,7 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
 	 * kernel is crippled.
 	 */
 	while (len) {
-		int sent = __write_console(data, len);
+		int sent = __write_console(cons, data, len);
 		
 		data += sent;
 		len -= sent;
@@ -114,9 +140,13 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
 
 static int domU_read_console(uint32_t vtermno, char *buf, int len)
 {
-	struct xencons_interface *intf = xencons_interface();
+	struct xencons_interface *intf;
 	XENCONS_RING_IDX cons, prod;
 	int recv = 0;
+	struct xencons_info *xencons = vtermno_to_xencons(vtermno);
+	if (xencons == NULL)
+		return -EINVAL;
+	intf = xencons->intf;
 
 	cons = intf->in_cons;
 	prod = intf->in_prod;
@@ -129,7 +159,7 @@ static int domU_read_console(uint32_t vtermno, char *buf, int len)
 	mb();			/* read ring before consuming */
 	intf->in_cons = cons;
 
-	notify_daemon();
+	notify_daemon(xencons);
 	return recv;
 }
 
@@ -172,33 +202,109 @@ static int xen_hvm_console_init(void)
 	int r;
 	uint64_t v = 0;
 	unsigned long mfn;
+	struct xencons_info *info;
 
 	if (!xen_hvm_domain())
 		return -ENODEV;
 
-	if (xencons_if != NULL)
-		return -EBUSY;
+	info = vtermno_to_xencons(HVC_COOKIE);
+	if (!info) {
+		info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
+		if (!info)
+			return -ENOMEM;
+	}
+
+	/* already configured */
+	if (info->intf != NULL)
+		return 0;
 
 	r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
-	if (r < 0)
+	if (r < 0) {
+		kfree(info);
 		return -ENODEV;
-	console_evtchn = v;
+	}
+	info->evtchn = v;
 	hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
-	if (r < 0)
+	if (r < 0) {
+		kfree(info);
 		return -ENODEV;
+	}
 	mfn = v;
-	xencons_if = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
-	if (xencons_if == NULL)
+	info->intf = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
+	if (info->intf == NULL) {
+		kfree(info);
+		return -ENODEV;
+	}
+	info->vtermno = HVC_COOKIE;
+
+	spin_lock(&xencons_lock);
+	list_add_tail(&info->list, &xenconsoles);
+	spin_unlock(&xencons_lock);
+
+	return 0;
+}
+
+static int xen_pv_console_init(void)
+{
+	struct xencons_info *info;
+
+	if (!xen_pv_domain())
 		return -ENODEV;
 
+	if (!xen_start_info->console.domU.evtchn)
+		return -ENODEV;
+
+	info = vtermno_to_xencons(HVC_COOKIE);
+	if (!info) {
+		info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
+		if (!info)
+			return -ENOMEM;
+	}
+
+	/* already configured */
+	if (info->intf != NULL)
+		return 0;
+
+	info->evtchn = xen_start_info->console.domU.evtchn;
+	info->intf = mfn_to_virt(xen_start_info->console.domU.mfn);
+	info->vtermno = HVC_COOKIE;
+
+	spin_lock(&xencons_lock);
+	list_add_tail(&info->list, &xenconsoles);
+	spin_unlock(&xencons_lock);
+
+	return 0;
+}
+
+static int xen_initial_domain_console_init(void)
+{
+	struct xencons_info *info;
+
+	if (!xen_initial_domain())
+		return -ENODEV;
+
+	info = vtermno_to_xencons(HVC_COOKIE);
+	if (!info) {
+		info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
+		if (!info)
+			return -ENOMEM;
+	}
+
+	info->irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
+	info->vtermno = HVC_COOKIE;
+
+	spin_lock(&xencons_lock);
+	list_add_tail(&info->list, &xenconsoles);
+	spin_unlock(&xencons_lock);
+
 	return 0;
 }
 
 static int __init xen_hvc_init(void)
 {
-	struct hvc_struct *hp;
-	struct hv_ops *ops;
 	int r;
+	struct xencons_info *info;
+	const struct hv_ops *ops;
 
 	if (!xen_domain())
 		return -ENODEV;
@@ -209,43 +315,251 @@ static int __init xen_hvc_init(void)
 
 	if (xen_initial_domain()) {
 		ops = &dom0_hvc_ops;
-		xencons_irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
+		r = xen_initial_domain_console_init();
+		if (r < 0)
+			return r;
+		info = vtermno_to_xencons(HVC_COOKIE);
 	} else {
 		ops = &domU_hvc_ops;
-		if (xen_pv_domain()) {
-			console_pfn = mfn_to_pfn(xen_start_info->console.domU.mfn);
-			console_evtchn = xen_start_info->console.domU.evtchn;
-		} else {
+		if (xen_hvm_domain())
 			r = xen_hvm_console_init();
-			if (r < 0)
-				return r;
-		}
-		xencons_irq = bind_evtchn_to_irq(console_evtchn);
-		if (xencons_irq < 0)
-			xencons_irq = 0; /* NO_IRQ */
 		else
-			irq_set_noprobe(xencons_irq);
+			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;
 	}
 
-	hp = hvc_alloc(HVC_COOKIE, xencons_irq, ops, 256);
-	if (IS_ERR(hp))
-		return PTR_ERR(hp);
+	return xenbus_register_frontend(&xencons_driver);
+}
 
-	hvc = hp;
+void xen_console_resume(void)
+{
+	struct xencons_info *info = vtermno_to_xencons(HVC_COOKIE);
+	if (info != NULL && info->irq)
+		rebind_evtchn_irq(info->evtchn, info->irq);
+}
 
+static void xencons_disconnect_backend(struct xencons_info *info)
+{
+	if (info->irq > 0)
+		unbind_from_irqhandler(info->irq, NULL);
+	info->irq = 0;
+	if (info->evtchn > 0)
+		xenbus_free_evtchn(info->xbdev, info->evtchn);
+	info->evtchn = 0;
+	if (info->gntref > 0)
+		gnttab_free_grant_references(info->gntref);
+	info->gntref = 0;
+	if (info->hvc != NULL)
+		hvc_remove(info->hvc);
+	info->hvc = NULL;
+}
+
+static void xencons_free(struct xencons_info *info)
+{
+	xencons_disconnect_backend(info);
+	free_page((unsigned long)info->intf);
+	info->intf = NULL;
+	info->vtermno = 0;
+	kfree(info);
+}
+
+static int xencons_remove(struct xenbus_device *dev)
+{
+	struct xencons_info *info = dev_get_drvdata(&dev->dev);
+
+	spin_lock(&xencons_lock);
+	list_del(&info->list);
+	spin_unlock(&xencons_lock);
+	xencons_free(info);
 	return 0;
 }
 
-void xen_console_resume(void)
+static int xencons_connect_backend(struct xenbus_device *dev,
+				  struct xencons_info *info)
+{
+	int ret, evtchn, devid, ref, irq;
+	struct xenbus_transaction xbt;
+	grant_ref_t gref_head;
+	unsigned long mfn;
+
+	ret = xenbus_alloc_evtchn(dev, &evtchn);
+	if (ret)
+		return ret;
+	info->evtchn = evtchn;
+	irq = bind_evtchn_to_irq(evtchn);
+	if (irq < 0)
+		return irq;
+	info->irq = irq;
+	devid = dev->nodename[strlen(dev->nodename) - 1] - '0';
+	info->hvc = hvc_alloc(xenbus_devid_to_vtermno(devid),
+			irq, &domU_hvc_ops, 256);
+	if (IS_ERR(info->hvc))
+		return PTR_ERR(info->hvc);
+	if (xen_pv_domain())
+		mfn = virt_to_mfn(info->intf);
+	else
+		mfn = __pa(info->intf) >> PAGE_SHIFT;
+	ret = gnttab_alloc_grant_references(1, &gref_head);
+	if (ret < 0)
+		return ret;
+	info->gntref = gref_head;
+	ref = gnttab_claim_grant_reference(&gref_head);
+	if (ref < 0)
+		return ref;
+	gnttab_grant_foreign_access_ref(ref, info->xbdev->otherend_id,
+			mfn, 0);
+
+ again:
+	ret = xenbus_transaction_start(&xbt);
+	if (ret) {
+		xenbus_dev_fatal(dev, ret, "starting transaction");
+		return ret;
+	}
+	ret = xenbus_printf(xbt, dev->nodename, "ring-ref", "%d", ref);
+	if (ret)
+		goto error_xenbus;
+	ret = xenbus_printf(xbt, dev->nodename, "port", "%u",
+			    evtchn);
+	if (ret)
+		goto error_xenbus;
+	ret = xenbus_printf(xbt, dev->nodename, "type", "ioemu");
+	if (ret)
+		goto error_xenbus;
+	ret = xenbus_transaction_end(xbt, 0);
+	if (ret) {
+		if (ret == -EAGAIN)
+			goto again;
+		xenbus_dev_fatal(dev, ret, "completing transaction");
+		return ret;
+	}
+
+	xenbus_switch_state(dev, XenbusStateInitialised);
+	return 0;
+
+ error_xenbus:
+	xenbus_transaction_end(xbt, 1);
+	xenbus_dev_fatal(dev, ret, "writing xenstore");
+	return ret;
+}
+
+static int __devinit xencons_probe(struct xenbus_device *dev,
+				  const struct xenbus_device_id *id)
 {
-	if (xencons_irq)
-		rebind_evtchn_irq(console_evtchn, xencons_irq);
+	int ret, devid;
+	struct xencons_info *info;
+
+	devid = dev->nodename[strlen(dev->nodename) - 1] - '0';
+	if (devid == 0)
+		return -ENODEV;
+
+	info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
+	if (!info)
+		goto error_nomem;
+	dev_set_drvdata(&dev->dev, info);
+	info->xbdev = dev;
+	info->vtermno = xenbus_devid_to_vtermno(devid);
+	info->intf = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
+	if (!info->intf)
+		goto error_nomem;
+
+	ret = xencons_connect_backend(dev, info);
+	if (ret < 0)
+		goto error;
+	spin_lock(&xencons_lock);
+	list_add_tail(&info->list, &xenconsoles);
+	spin_unlock(&xencons_lock);
+
+	return 0;
+
+ error_nomem:
+	ret = -ENOMEM;
+	xenbus_dev_fatal(dev, ret, "allocating device memory");
+ error:
+	xencons_free(info);
+	return ret;
+}
+
+static int xencons_resume(struct xenbus_device *dev)
+{
+	struct xencons_info *info = dev_get_drvdata(&dev->dev);
+
+	xencons_disconnect_backend(info);
+	memset(info->intf, 0, PAGE_SIZE);
+	return xencons_connect_backend(dev, info);
 }
 
+static void xencons_backend_changed(struct xenbus_device *dev,
+				   enum xenbus_state backend_state)
+{
+	switch (backend_state) {
+	case XenbusStateReconfiguring:
+	case XenbusStateReconfigured:
+	case XenbusStateInitialising:
+	case XenbusStateInitialised:
+	case XenbusStateUnknown:
+	case XenbusStateClosed:
+		break;
+
+	case XenbusStateInitWait:
+		break;
+
+	case XenbusStateConnected:
+		xenbus_switch_state(dev, XenbusStateConnected);
+		break;
+
+	case XenbusStateClosing:
+		xenbus_frontend_closed(dev);
+		break;
+	}
+}
+
+static const struct xenbus_device_id xencons_ids[] = {
+	{ "console" },
+	{ "" }
+};
+
+
 static void __exit xen_hvc_fini(void)
 {
-	if (hvc)
-		hvc_remove(hvc);
+	struct xencons_info *entry, *next;
+
+	if (list_empty(&xenconsoles))
+			return;
+
+	spin_lock(&xencons_lock);
+	list_for_each_entry_safe(entry, next, &xenconsoles, list) {
+		list_del(&entry->list);
+		if (entry->xbdev)
+			xencons_remove(entry->xbdev);
+		else {
+			if (entry->irq > 0)
+				unbind_from_irqhandler(entry->irq, NULL);
+			if (entry->hvc);
+				hvc_remove(entry->hvc);
+			kfree(entry);
+		}
+	}
+	spin_unlock(&xencons_lock);
 }
 
 static int xen_cons_init(void)
@@ -258,18 +572,31 @@ static int xen_cons_init(void)
 	if (xen_initial_domain())
 		ops = &dom0_hvc_ops;
 	else {
+		int r;
 		ops = &domU_hvc_ops;
 
-		if (xen_pv_domain())
-			console_evtchn = xen_start_info->console.domU.evtchn;
+		if (xen_hvm_domain())
+			r = xen_hvm_console_init();
 		else
-			xen_hvm_console_init();
+			r = xen_pv_console_init();
+		if (r < 0)
+			return r;
 	}
 
 	hvc_instantiate(HVC_COOKIE, 0, ops);
 	return 0;
 }
 
+static struct xenbus_driver xencons_driver = {
+	.name = "xenconsole",
+	.owner = THIS_MODULE,
+	.ids = xencons_ids,
+	.probe = xencons_probe,
+	.remove = xencons_remove,
+	.resume = xencons_resume,
+	.otherend_changed = xencons_backend_changed,
+};
+
 module_init(xen_hvc_init);
 module_exit(xen_hvc_fini);
 console_initcall(xen_cons_init);
-- 
1.7.2.5

Re: [PATCH 1/2] hvc_xen: support PV on HVM consoles

[Konrad Rzeszutek Wilk]

On Thu, Jan 26, 2012 at 12:43:26PM +0000, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  drivers/tty/hvc/hvc_xen.c          |   86 +++++++++++++++++++++++++++++-------
>  include/xen/interface/hvm/params.h |    6 ++-
>  2 files changed, 75 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 52fdf60..dd6641f 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -24,9 +24,12 @@
>  #include <linux/init.h>
>  #include <linux/types.h>
>  
> +#include <asm/io.h>
>  #include <asm/xen/hypervisor.h>
>  
>  #include <xen/xen.h>
> +#include <xen/interface/xen.h>
> +#include <xen/hvm.h>
>  #include <xen/page.h>
>  #include <xen/events.h>
>  #include <xen/interface/io/console.h>
> @@ -42,9 +45,13 @@ static int xencons_irq;
>  /* ------------------------------------------------------------------ */
>  
>  static unsigned long console_pfn = ~0ul;
> +static unsigned int console_evtchn = ~0ul;
> +static struct xencons_interface *xencons_if = NULL;
>  
>  static inline struct xencons_interface *xencons_interface(void)
>  {
> +	if (xencons_if != NULL)
> +		return xencons_if;
>  	if (console_pfn == ~0ul)
>  		return mfn_to_virt(xen_start_info->console.domU.mfn);
>  	else
> @@ -54,7 +61,10 @@ static inline struct xencons_interface *xencons_interface(void)
>  static inline void notify_daemon(void)
>  {
>  	/* Use evtchn: this is called early, before irq is set up. */
> -	notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> +	if (console_evtchn == ~0ul)
> +		notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> +	else
> +		notify_remote_via_evtchn(console_evtchn);
>  }
>  
>  static int __write_console(const char *data, int len)
> @@ -157,28 +167,65 @@ static struct hv_ops dom0_hvc_ops = {
>  	.notifier_hangup = notifier_hangup_irq,
>  };
>  
> +static int xen_hvm_console_init(void)
> +{
> +	int r;
> +	uint64_t v = 0;
> +	unsigned long mfn;
> +
> +	if (!xen_hvm_domain())
> +		return -ENODEV;
> +
> +	if (xencons_if != NULL)
> +		return -EBUSY;
> +
> +	r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
> +	if (r < 0)
> +		return -ENODEV;
> +	console_evtchn = v;
> +	hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> +	if (r < 0)
> +		return -ENODEV;

If we fail here, the console_evtchn is still going to be set
meaning that "notify_daemon"  will use the event channel. Thought
I can't think of the notify daemon being called after the init had
failed so this might not be an issue.

> +	mfn = v;
> +	xencons_if = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
> +	if (xencons_if == NULL)
> +		return -ENODEV;

Ditto. If we fail, we have the 'console_evtchn' set to something
else than ~0UL.

> +
> +	return 0;
> +}
> +
>  static int __init xen_hvc_init(void)
>  {
>  	struct hvc_struct *hp;
>  	struct hv_ops *ops;
> +	int r;
>  
> -	if (!xen_pv_domain())
> +	if (!xen_domain())
> +		return -ENODEV;
> +
> +	if (xen_pv_domain() && !xen_initial_domain() &&
> +			!xen_start_info->console.domU.evtchn)

Ewww.. What about just leaving the check:
>  		return -ENODEV;
>  
>  	if (xen_initial_domain()) {
>  		ops = &dom0_hvc_ops;
>  		xencons_irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
>  	} else {
> -		if (!xen_start_info->console.domU.evtchn)
> -			return -ENODEV;

this one as 'if (xen_pv_domain()) && !xen-..)

That might make the code nicer to read?

> -
>  		ops = &domU_hvc_ops;
> -		xencons_irq = bind_evtchn_to_irq(xen_start_info->console.domU.evtchn);
> +		if (xen_pv_domain()) {
> +			console_pfn = mfn_to_pfn(xen_start_info->console.domU.mfn);
> +			console_evtchn = xen_start_info->console.domU.evtchn;
> +		} else {
> +			r = xen_hvm_console_init();
> +			if (r < 0)
> +				return r;
> +		}
> +		xencons_irq = bind_evtchn_to_irq(console_evtchn);
> +		if (xencons_irq < 0)
> +			xencons_irq = 0; /* NO_IRQ */
> +		else
> +			irq_set_noprobe(xencons_irq);
>  	}
> -	if (xencons_irq < 0)
> -		xencons_irq = 0; /* NO_IRQ */
> -	else
> -		irq_set_noprobe(xencons_irq);
>  
>  	hp = hvc_alloc(HVC_COOKIE, xencons_irq, ops, 256);
>  	if (IS_ERR(hp))
> @@ -186,15 +233,13 @@ static int __init xen_hvc_init(void)
>  
>  	hvc = hp;
>  
> -	console_pfn = mfn_to_pfn(xen_start_info->console.domU.mfn);
> -
>  	return 0;
>  }
>  
>  void xen_console_resume(void)
>  {
>  	if (xencons_irq)
> -		rebind_evtchn_irq(xen_start_info->console.domU.evtchn, xencons_irq);
> +		rebind_evtchn_irq(console_evtchn, xencons_irq);
>  }
>  
>  static void __exit xen_hvc_fini(void)
> @@ -205,16 +250,22 @@ static void __exit xen_hvc_fini(void)
>  
>  static int xen_cons_init(void)
>  {
> -	struct hv_ops *ops;
> +	const struct hv_ops *ops;
>  
> -	if (!xen_pv_domain())
> +	if (!xen_domain())
>  		return 0;
>  
>  	if (xen_initial_domain())
>  		ops = &dom0_hvc_ops;
> -	else
> +	else {
>  		ops = &domU_hvc_ops;
>  
> +		if (xen_pv_domain())
> +			console_evtchn = xen_start_info->console.domU.evtchn;
> +		else
> +			xen_hvm_console_init();
> +	}
> +
>  	hvc_instantiate(HVC_COOKIE, 0, ops);
>  	return 0;
>  }
> @@ -230,6 +281,9 @@ static void xenboot_write_console(struct console *console, const char *string,
>  	unsigned int linelen, off = 0;
>  	const char *pos;
>  
> +	if (!xen_pv_domain())
> +		return;
> +
>  	dom0_write_console(0, string, len);
>  
>  	if (xen_initial_domain())
> diff --git a/include/xen/interface/hvm/params.h b/include/xen/interface/hvm/params.h
> index 1888d8c..1b4f923 100644
> --- a/include/xen/interface/hvm/params.h
> +++ b/include/xen/interface/hvm/params.h
> @@ -90,6 +90,10 @@
>  /* Boolean: Enable aligning all periodic vpts to reduce interrupts */
>  #define HVM_PARAM_VPT_ALIGN    16
>  
> -#define HVM_NR_PARAMS          17
> +/* Console debug shared memory ring and event channel */
> +#define HVM_PARAM_CONSOLE_PFN    17
> +#define HVM_PARAM_CONSOLE_EVTCHN 18
> +
> +#define HVM_NR_PARAMS          19
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
> -- 
> 1.7.2.5

Re: [PATCH 1/2] hvc_xen: support PV on HVM consoles

[Stefano Stabellini]

On Fri, 27 Jan 2012, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 26, 2012 at 12:43:26PM +0000, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  drivers/tty/hvc/hvc_xen.c          |   86 +++++++++++++++++++++++++++++-------
> >  include/xen/interface/hvm/params.h |    6 ++-
> >  2 files changed, 75 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> > index 52fdf60..dd6641f 100644
> > --- a/drivers/tty/hvc/hvc_xen.c
> > +++ b/drivers/tty/hvc/hvc_xen.c
> > @@ -24,9 +24,12 @@
> >  #include <linux/init.h>
> >  #include <linux/types.h>
> >  
> > +#include <asm/io.h>
> >  #include <asm/xen/hypervisor.h>
> >  
> >  #include <xen/xen.h>
> > +#include <xen/interface/xen.h>
> > +#include <xen/hvm.h>
> >  #include <xen/page.h>
> >  #include <xen/events.h>
> >  #include <xen/interface/io/console.h>
> > @@ -42,9 +45,13 @@ static int xencons_irq;
> >  /* ------------------------------------------------------------------ */
> >  
> >  static unsigned long console_pfn = ~0ul;
> > +static unsigned int console_evtchn = ~0ul;
> > +static struct xencons_interface *xencons_if = NULL;
> >  
> >  static inline struct xencons_interface *xencons_interface(void)
> >  {
> > +	if (xencons_if != NULL)
> > +		return xencons_if;
> >  	if (console_pfn == ~0ul)
> >  		return mfn_to_virt(xen_start_info->console.domU.mfn);
> >  	else
> > @@ -54,7 +61,10 @@ static inline struct xencons_interface *xencons_interface(void)
> >  static inline void notify_daemon(void)
> >  {
> >  	/* Use evtchn: this is called early, before irq is set up. */
> > -	notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> > +	if (console_evtchn == ~0ul)
> > +		notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> > +	else
> > +		notify_remote_via_evtchn(console_evtchn);
> >  }
> >  
> >  static int __write_console(const char *data, int len)
> > @@ -157,28 +167,65 @@ static struct hv_ops dom0_hvc_ops = {
> >  	.notifier_hangup = notifier_hangup_irq,
> >  };
> >  
> > +static int xen_hvm_console_init(void)
> > +{
> > +	int r;
> > +	uint64_t v = 0;
> > +	unsigned long mfn;
> > +
> > +	if (!xen_hvm_domain())
> > +		return -ENODEV;
> > +
> > +	if (xencons_if != NULL)
> > +		return -EBUSY;
> > +
> > +	r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
> > +	if (r < 0)
> > +		return -ENODEV;
> > +	console_evtchn = v;
> > +	hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> > +	if (r < 0)
> > +		return -ENODEV;
> 
> If we fail here, the console_evtchn is still going to be set
> meaning that "notify_daemon"  will use the event channel. Thought
> I can't think of the notify daemon being called after the init had
> failed so this might not be an issue.
> 
> > +	mfn = v;
> > +	xencons_if = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
> > +	if (xencons_if == NULL)
> > +		return -ENODEV;
> 
> Ditto. If we fail, we have the 'console_evtchn' set to something
> else than ~0UL.

Like you wrote, I think it doesn't matter.



> > +	return 0;
> > +}
> > +
> >  static int __init xen_hvc_init(void)
> >  {
> >  	struct hvc_struct *hp;
> >  	struct hv_ops *ops;
> > +	int r;
> >  
> > -	if (!xen_pv_domain())
> > +	if (!xen_domain())
> > +		return -ENODEV;
> > +
> > +	if (xen_pv_domain() && !xen_initial_domain() &&
> > +			!xen_start_info->console.domU.evtchn)
> 
> Ewww.. What about just leaving the check:
> >  		return -ENODEV;
> >  
> >  	if (xen_initial_domain()) {
> >  		ops = &dom0_hvc_ops;
> >  		xencons_irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
> >  	} else {
> > -		if (!xen_start_info->console.domU.evtchn)
> > -			return -ENODEV;
> 
> this one as 'if (xen_pv_domain()) && !xen-..)
> 
> That might make the code nicer to read?

Yes, good idea.

Re: [Xen-devel] [PATCH 2/2] hvc_xen: implement multiconsole support

[Konrad Rzeszutek Wilk]

On Thu, Jan 26, 2012 at 12:43:27PM +0000, Stefano Stabellini wrote:
> This patch implements support for multiple consoles:
> consoles other than the first one are setup using the traditional xenbus
> and grant-table based mechanism.
> We use a list to keep track of the allocated consoles, we don't
> expect too many of them anyway.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  drivers/tty/hvc/hvc_xen.c |  439 +++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 383 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index dd6641f..97732fb 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -23,6 +23,7 @@
>  #include <linux/err.h>
>  #include <linux/init.h>
>  #include <linux/types.h>
> +#include <linux/list.h>
>  
>  #include <asm/io.h>
>  #include <asm/xen/hypervisor.h>
> @@ -30,47 +31,69 @@
>  #include <xen/xen.h>
>  #include <xen/interface/xen.h>
>  #include <xen/hvm.h>
> +#include <xen/grant_table.h>
>  #include <xen/page.h>
>  #include <xen/events.h>
>  #include <xen/interface/io/console.h>
>  #include <xen/hvc-console.h>
> +#include <xen/xenbus.h>
>  
>  #include "hvc_console.h"
>  
>  #define HVC_COOKIE   0x58656e /* "Xen" in hex */
>  
> -static struct hvc_struct *hvc;
> -static int xencons_irq;
> +struct xencons_info {
> +	struct list_head list;
> +	struct xenbus_device *xbdev;
> +	struct xencons_interface *intf;
> +	unsigned int evtchn;
> +	struct hvc_struct *hvc;
> +	int irq;
> +	int vtermno;
> +	grant_ref_t gntref;
> +};
> +
> +static LIST_HEAD(xenconsoles);
> +static DEFINE_SPINLOCK(xencons_lock);
> +static struct xenbus_driver xencons_driver;
>  
>  /* ------------------------------------------------------------------ */
>  
> -static unsigned long console_pfn = ~0ul;
> -static unsigned int console_evtchn = ~0ul;
> -static struct xencons_interface *xencons_if = NULL;
> +static struct xencons_info *vtermno_to_xencons(int vtermno)
> +{
> +	struct xencons_info *entry, *ret = NULL;
> +
> +	if (list_empty(&xenconsoles))
> +			return NULL;
>  
> -static inline struct xencons_interface *xencons_interface(void)
> +	spin_lock(&xencons_lock);

This spinlock gets hit everytime something is typed or written on the
console right? Isn't there an spinlock already in the hvc driver...

Or are we protected by the vtermnos being checked for -1?


> +	list_for_each_entry(entry, &xenconsoles, list) {
> +		if (entry->vtermno == vtermno) {
> +			ret  = entry;
> +			break;
> +		}
> +	}
> +	spin_unlock(&xencons_lock);
> +
> +	return ret;
> +}
> +
> +static inline int xenbus_devid_to_vtermno(int devid)
>  {
> -	if (xencons_if != NULL)
> -		return xencons_if;
> -	if (console_pfn == ~0ul)
> -		return mfn_to_virt(xen_start_info->console.domU.mfn);
> -	else
> -		return __va(console_pfn << PAGE_SHIFT);
> +	return devid + HVC_COOKIE;
>  }
>  
> -static inline void notify_daemon(void)
> +static inline void notify_daemon(struct xencons_info *cons)
>  {
>  	/* Use evtchn: this is called early, before irq is set up. */
> -	if (console_evtchn == ~0ul)
> -		notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> -	else
> -		notify_remote_via_evtchn(console_evtchn);
> +	notify_remote_via_evtchn(cons->evtchn);
>  }
>  
> -static int __write_console(const char *data, int len)
> +static int __write_console(struct xencons_info *xencons,
> +		const char *data, int len)
>  {
> -	struct xencons_interface *intf = xencons_interface();
>  	XENCONS_RING_IDX cons, prod;
> +	struct xencons_interface *intf = xencons->intf;
>  	int sent = 0;
>  
>  	cons = intf->out_cons;
> @@ -85,13 +108,16 @@ static int __write_console(const char *data, int len)
>  	intf->out_prod = prod;
>  
>  	if (sent)
> -		notify_daemon();
> +		notify_daemon(xencons);
>  	return sent;
>  }
>  
>  static int domU_write_console(uint32_t vtermno, const char *data, int len)
>  {
>  	int ret = len;
> +	struct xencons_info *cons = vtermno_to_xencons(vtermno);
> +	if (cons == NULL)
> +		return -EINVAL;
>  
>  	/*
>  	 * Make sure the whole buffer is emitted, polling if
> @@ -100,7 +126,7 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
>  	 * kernel is crippled.
>  	 */
>  	while (len) {
> -		int sent = __write_console(data, len);
> +		int sent = __write_console(cons, data, len);
>  		
>  		data += sent;
>  		len -= sent;
> @@ -114,9 +140,13 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
>  
>  static int domU_read_console(uint32_t vtermno, char *buf, int len)
>  {
> -	struct xencons_interface *intf = xencons_interface();
> +	struct xencons_interface *intf;
>  	XENCONS_RING_IDX cons, prod;
>  	int recv = 0;
> +	struct xencons_info *xencons = vtermno_to_xencons(vtermno);
> +	if (xencons == NULL)
> +		return -EINVAL;
> +	intf = xencons->intf;
>  
>  	cons = intf->in_cons;
>  	prod = intf->in_prod;
> @@ -129,7 +159,7 @@ static int domU_read_console(uint32_t vtermno, char *buf, int len)
>  	mb();			/* read ring before consuming */
>  	intf->in_cons = cons;
>  
> -	notify_daemon();
> +	notify_daemon(xencons);
>  	return recv;
>  }
>  
> @@ -172,33 +202,109 @@ static int xen_hvm_console_init(void)
>  	int r;
>  	uint64_t v = 0;
>  	unsigned long mfn;
> +	struct xencons_info *info;
>  
>  	if (!xen_hvm_domain())
>  		return -ENODEV;
>  
> -	if (xencons_if != NULL)
> -		return -EBUSY;
> +	info = vtermno_to_xencons(HVC_COOKIE);
> +	if (!info) {
> +		info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
> +		if (!info)
> +			return -ENOMEM;
> +	}
> +
> +	/* already configured */
> +	if (info->intf != NULL)
> +		return 0;
>  
>  	r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
> -	if (r < 0)
> +	if (r < 0) {
> +		kfree(info);
>  		return -ENODEV;
> -	console_evtchn = v;
> +	}
> +	info->evtchn = v;
>  	hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> -	if (r < 0)
> +	if (r < 0) {
> +		kfree(info);
>  		return -ENODEV;
> +	}
>  	mfn = v;
> -	xencons_if = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
> -	if (xencons_if == NULL)
> +	info->intf = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
> +	if (info->intf == NULL) {
> +		kfree(info);
> +		return -ENODEV;
> +	}
> +	info->vtermno = HVC_COOKIE;
> +
> +	spin_lock(&xencons_lock);
> +	list_add_tail(&info->list, &xenconsoles);
> +	spin_unlock(&xencons_lock);
> +
> +	return 0;
> +}
> +
> +static int xen_pv_console_init(void)
> +{
> +	struct xencons_info *info;
> +
> +	if (!xen_pv_domain())
>  		return -ENODEV;
>  
> +	if (!xen_start_info->console.domU.evtchn)
> +		return -ENODEV;
> +
> +	info = vtermno_to_xencons(HVC_COOKIE);
> +	if (!info) {
> +		info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);

Ugh. Use kzalloc here. Especially as you are testing it below (and
kmalloc with certain CONFIG_DEBUG.. can make the the returned memory
have bogus data.

> +		if (!info)
> +			return -ENOMEM;
> +	}
> +
> +	/* already configured */
> +	if (info->intf != NULL)
> +		return 0;
> +
> +	info->evtchn = xen_start_info->console.domU.evtchn;
> +	info->intf = mfn_to_virt(xen_start_info->console.domU.mfn);
> +	info->vtermno = HVC_COOKIE;
> +
> +	spin_lock(&xencons_lock);
> +	list_add_tail(&info->list, &xenconsoles);
> +	spin_unlock(&xencons_lock);
> +
> +	return 0;
> +}
> +
> +static int xen_initial_domain_console_init(void)
> +{
> +	struct xencons_info *info;
> +
> +	if (!xen_initial_domain())
> +		return -ENODEV;
> +
> +	info = vtermno_to_xencons(HVC_COOKIE);
> +	if (!info) {
> +		info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);

Ditto
> +		if (!info)
> +			return -ENOMEM;
> +	}
> +
> +	info->irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
> +	info->vtermno = HVC_COOKIE;
> +
> +	spin_lock(&xencons_lock);
> +	list_add_tail(&info->list, &xenconsoles);
> +	spin_unlock(&xencons_lock);
> +
>  	return 0;
>  }
>  
>  static int __init xen_hvc_init(void)
>  {
> -	struct hvc_struct *hp;
> -	struct hv_ops *ops;
>  	int r;
> +	struct xencons_info *info;
> +	const struct hv_ops *ops;
>  
>  	if (!xen_domain())
>  		return -ENODEV;
> @@ -209,43 +315,251 @@ static int __init xen_hvc_init(void)
>  
>  	if (xen_initial_domain()) {
>  		ops = &dom0_hvc_ops;
> -		xencons_irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
> +		r = xen_initial_domain_console_init();
> +		if (r < 0)
> +			return r;
> +		info = vtermno_to_xencons(HVC_COOKIE);
>  	} else {
>  		ops = &domU_hvc_ops;
> -		if (xen_pv_domain()) {
> -			console_pfn = mfn_to_pfn(xen_start_info->console.domU.mfn);
> -			console_evtchn = xen_start_info->console.domU.evtchn;
> -		} else {
> +		if (xen_hvm_domain())
>  			r = xen_hvm_console_init();
> -			if (r < 0)
> -				return r;
> -		}
> -		xencons_irq = bind_evtchn_to_irq(console_evtchn);
> -		if (xencons_irq < 0)
> -			xencons_irq = 0; /* NO_IRQ */
>  		else
> -			irq_set_noprobe(xencons_irq);
> +			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;
>  	}
>  
> -	hp = hvc_alloc(HVC_COOKIE, xencons_irq, ops, 256);
> -	if (IS_ERR(hp))
> -		return PTR_ERR(hp);
> +	return xenbus_register_frontend(&xencons_driver);
> +}
>  
> -	hvc = hp;
> +void xen_console_resume(void)
> +{
> +	struct xencons_info *info = vtermno_to_xencons(HVC_COOKIE);
> +	if (info != NULL && info->irq)
> +		rebind_evtchn_irq(info->evtchn, info->irq);
> +}
>  
> +static void xencons_disconnect_backend(struct xencons_info *info)
> +{
> +	if (info->irq > 0)
> +		unbind_from_irqhandler(info->irq, NULL);
> +	info->irq = 0;
> +	if (info->evtchn > 0)
> +		xenbus_free_evtchn(info->xbdev, info->evtchn);
> +	info->evtchn = 0;
> +	if (info->gntref > 0)
> +		gnttab_free_grant_references(info->gntref);
> +	info->gntref = 0;
> +	if (info->hvc != NULL)
> +		hvc_remove(info->hvc);
> +	info->hvc = NULL;
> +}
> +
> +static void xencons_free(struct xencons_info *info)
> +{
> +	xencons_disconnect_backend(info);
> +	free_page((unsigned long)info->intf);
> +	info->intf = NULL;
> +	info->vtermno = 0;
> +	kfree(info);
> +}
> +
> +static int xencons_remove(struct xenbus_device *dev)
> +{
> +	struct xencons_info *info = dev_get_drvdata(&dev->dev);
> +
> +	spin_lock(&xencons_lock);
> +	list_del(&info->list);
> +	spin_unlock(&xencons_lock);
> +	xencons_free(info);
>  	return 0;
>  }
>  
> -void xen_console_resume(void)
> +static int xencons_connect_backend(struct xenbus_device *dev,
> +				  struct xencons_info *info)
> +{
> +	int ret, evtchn, devid, ref, irq;
> +	struct xenbus_transaction xbt;
> +	grant_ref_t gref_head;
> +	unsigned long mfn;
> +
> +	ret = xenbus_alloc_evtchn(dev, &evtchn);
> +	if (ret)
> +		return ret;
> +	info->evtchn = evtchn;
> +	irq = bind_evtchn_to_irq(evtchn);
> +	if (irq < 0)
> +		return irq;
> +	info->irq = irq;
> +	devid = dev->nodename[strlen(dev->nodename) - 1] - '0';
> +	info->hvc = hvc_alloc(xenbus_devid_to_vtermno(devid),
> +			irq, &domU_hvc_ops, 256);
> +	if (IS_ERR(info->hvc))
> +		return PTR_ERR(info->hvc);
> +	if (xen_pv_domain())
> +		mfn = virt_to_mfn(info->intf);
> +	else
> +		mfn = __pa(info->intf) >> PAGE_SHIFT;
> +	ret = gnttab_alloc_grant_references(1, &gref_head);
> +	if (ret < 0)
> +		return ret;
> +	info->gntref = gref_head;
> +	ref = gnttab_claim_grant_reference(&gref_head);
> +	if (ref < 0)
> +		return ref;
> +	gnttab_grant_foreign_access_ref(ref, info->xbdev->otherend_id,
> +			mfn, 0);
> +
> + again:
> +	ret = xenbus_transaction_start(&xbt);
> +	if (ret) {
> +		xenbus_dev_fatal(dev, ret, "starting transaction");
> +		return ret;
> +	}
> +	ret = xenbus_printf(xbt, dev->nodename, "ring-ref", "%d", ref);
> +	if (ret)
> +		goto error_xenbus;
> +	ret = xenbus_printf(xbt, dev->nodename, "port", "%u",
> +			    evtchn);
> +	if (ret)
> +		goto error_xenbus;
> +	ret = xenbus_printf(xbt, dev->nodename, "type", "ioemu");
> +	if (ret)
> +		goto error_xenbus;
> +	ret = xenbus_transaction_end(xbt, 0);
> +	if (ret) {
> +		if (ret == -EAGAIN)
> +			goto again;
> +		xenbus_dev_fatal(dev, ret, "completing transaction");
> +		return ret;
> +	}
> +
> +	xenbus_switch_state(dev, XenbusStateInitialised);
> +	return 0;
> +
> + error_xenbus:
> +	xenbus_transaction_end(xbt, 1);
> +	xenbus_dev_fatal(dev, ret, "writing xenstore");
> +	return ret;
> +}
> +
> +static int __devinit xencons_probe(struct xenbus_device *dev,
> +				  const struct xenbus_device_id *id)
>  {
> -	if (xencons_irq)
> -		rebind_evtchn_irq(console_evtchn, xencons_irq);
> +	int ret, devid;
> +	struct xencons_info *info;
> +
> +	devid = dev->nodename[strlen(dev->nodename) - 1] - '0';
> +	if (devid == 0)
> +		return -ENODEV;
> +
> +	info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
> +	if (!info)
> +		goto error_nomem;
> +	dev_set_drvdata(&dev->dev, info);
> +	info->xbdev = dev;
> +	info->vtermno = xenbus_devid_to_vtermno(devid);
> +	info->intf = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> +	if (!info->intf)
> +		goto error_nomem;
> +
> +	ret = xencons_connect_backend(dev, info);
> +	if (ret < 0)
> +		goto error;
> +	spin_lock(&xencons_lock);
> +	list_add_tail(&info->list, &xenconsoles);
> +	spin_unlock(&xencons_lock);
> +
> +	return 0;
> +
> + error_nomem:
> +	ret = -ENOMEM;
> +	xenbus_dev_fatal(dev, ret, "allocating device memory");
> + error:
> +	xencons_free(info);
> +	return ret;
> +}
> +
> +static int xencons_resume(struct xenbus_device *dev)
> +{
> +	struct xencons_info *info = dev_get_drvdata(&dev->dev);
> +
> +	xencons_disconnect_backend(info);
> +	memset(info->intf, 0, PAGE_SIZE);
> +	return xencons_connect_backend(dev, info);
>  }
>  
> +static void xencons_backend_changed(struct xenbus_device *dev,
> +				   enum xenbus_state backend_state)
> +{
> +	switch (backend_state) {
> +	case XenbusStateReconfiguring:
> +	case XenbusStateReconfigured:
> +	case XenbusStateInitialising:
> +	case XenbusStateInitialised:
> +	case XenbusStateUnknown:
> +	case XenbusStateClosed:
> +		break;
> +
> +	case XenbusStateInitWait:
> +		break;
> +
> +	case XenbusStateConnected:
> +		xenbus_switch_state(dev, XenbusStateConnected);
> +		break;
> +
> +	case XenbusStateClosing:
> +		xenbus_frontend_closed(dev);
> +		break;
> +	}
> +}
> +
> +static const struct xenbus_device_id xencons_ids[] = {
> +	{ "console" },
> +	{ "" }
> +};
> +
> +
>  static void __exit xen_hvc_fini(void)
>  {
> -	if (hvc)
> -		hvc_remove(hvc);
> +	struct xencons_info *entry, *next;
> +
> +	if (list_empty(&xenconsoles))
> +			return;
> +
> +	spin_lock(&xencons_lock);
> +	list_for_each_entry_safe(entry, next, &xenconsoles, list) {
> +		list_del(&entry->list);
> +		if (entry->xbdev)
> +			xencons_remove(entry->xbdev);
> +		else {
> +			if (entry->irq > 0)
> +				unbind_from_irqhandler(entry->irq, NULL);
> +			if (entry->hvc);
> +				hvc_remove(entry->hvc);
> +			kfree(entry);
> +		}
> +	}
> +	spin_unlock(&xencons_lock);
>  }
>  
>  static int xen_cons_init(void)
> @@ -258,18 +572,31 @@ static int xen_cons_init(void)
>  	if (xen_initial_domain())
>  		ops = &dom0_hvc_ops;
>  	else {
> +		int r;
>  		ops = &domU_hvc_ops;
>  
> -		if (xen_pv_domain())
> -			console_evtchn = xen_start_info->console.domU.evtchn;
> +		if (xen_hvm_domain())
> +			r = xen_hvm_console_init();
>  		else
> -			xen_hvm_console_init();
> +			r = xen_pv_console_init();
> +		if (r < 0)
> +			return r;
>  	}
>  
>  	hvc_instantiate(HVC_COOKIE, 0, ops);
>  	return 0;
>  }
>  
> +static struct xenbus_driver xencons_driver = {
> +	.name = "xenconsole",
> +	.owner = THIS_MODULE,
> +	.ids = xencons_ids,
> +	.probe = xencons_probe,
> +	.remove = xencons_remove,
> +	.resume = xencons_resume,
> +	.otherend_changed = xencons_backend_changed,
> +};
> +
>  module_init(xen_hvc_init);
>  module_exit(xen_hvc_fini);
>  console_initcall(xen_cons_init);
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [Xen-devel] [PATCH 2/2] hvc_xen: implement multiconsole support

[Stefano Stabellini]

On Fri, 27 Jan 2012, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 26, 2012 at 12:43:27PM +0000, Stefano Stabellini wrote:
> > This patch implements support for multiple consoles:
> > consoles other than the first one are setup using the traditional xenbus
> > and grant-table based mechanism.
> > We use a list to keep track of the allocated consoles, we don't
> > expect too many of them anyway.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  drivers/tty/hvc/hvc_xen.c |  439 +++++++++++++++++++++++++++++++++++++++------
> >  1 files changed, 383 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> > index dd6641f..97732fb 100644
> > --- a/drivers/tty/hvc/hvc_xen.c
> > +++ b/drivers/tty/hvc/hvc_xen.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/err.h>
> >  #include <linux/init.h>
> >  #include <linux/types.h>
> > +#include <linux/list.h>
> >
> >  #include <asm/io.h>
> >  #include <asm/xen/hypervisor.h>
> > @@ -30,47 +31,69 @@
> >  #include <xen/xen.h>
> >  #include <xen/interface/xen.h>
> >  #include <xen/hvm.h>
> > +#include <xen/grant_table.h>
> >  #include <xen/page.h>
> >  #include <xen/events.h>
> >  #include <xen/interface/io/console.h>
> >  #include <xen/hvc-console.h>
> > +#include <xen/xenbus.h>
> >
> >  #include "hvc_console.h"
> >
> >  #define HVC_COOKIE   0x58656e /* "Xen" in hex */
> >
> > -static struct hvc_struct *hvc;
> > -static int xencons_irq;
> > +struct xencons_info {
> > +     struct list_head list;
> > +     struct xenbus_device *xbdev;
> > +     struct xencons_interface *intf;
> > +     unsigned int evtchn;
> > +     struct hvc_struct *hvc;
> > +     int irq;
> > +     int vtermno;
> > +     grant_ref_t gntref;
> > +};
> > +
> > +static LIST_HEAD(xenconsoles);
> > +static DEFINE_SPINLOCK(xencons_lock);
> > +static struct xenbus_driver xencons_driver;
> >
> >  /* ------------------------------------------------------------------ */
> >
> > -static unsigned long console_pfn = ~0ul;
> > -static unsigned int console_evtchn = ~0ul;
> > -static struct xencons_interface *xencons_if = NULL;
> > +static struct xencons_info *vtermno_to_xencons(int vtermno)
> > +{
> > +     struct xencons_info *entry, *ret = NULL;
> > +
> > +     if (list_empty(&xenconsoles))
> > +                     return NULL;
> >
> > -static inline struct xencons_interface *xencons_interface(void)
> > +     spin_lock(&xencons_lock);
> 
> This spinlock gets hit everytime something is typed or written on the
> console right? Isn't there an spinlock already in the hvc driver...
> 
> Or are we protected by the vtermnos being checked for -1?

I think you are right: the spinlock is there to protect access to the
list, so we can change list_for_each_entry to list_for_each_entry_safe
in vtermno_to_xencons and we solve the problem. All the other spinlock
acquisitions are done at console creation/destruction.


> > +     list_for_each_entry(entry, &xenconsoles, list) {
> > +             if (entry->vtermno == vtermno) {
> > +                     ret  = entry;
> > +                     break;
> > +             }
> > +     }
> > +     spin_unlock(&xencons_lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static inline int xenbus_devid_to_vtermno(int devid)
> >  {
> > -     if (xencons_if != NULL)
> > -             return xencons_if;
> > -     if (console_pfn == ~0ul)
> > -             return mfn_to_virt(xen_start_info->console.domU.mfn);
> > -     else
> > -             return __va(console_pfn << PAGE_SHIFT);
> > +     return devid + HVC_COOKIE;
> >  }
> >
> > -static inline void notify_daemon(void)
> > +static inline void notify_daemon(struct xencons_info *cons)
> >  {
> >       /* Use evtchn: this is called early, before irq is set up. */
> > -     if (console_evtchn == ~0ul)
> > -             notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> > -     else
> > -             notify_remote_via_evtchn(console_evtchn);
> > +     notify_remote_via_evtchn(cons->evtchn);
> >  }
> >
> > -static int __write_console(const char *data, int len)
> > +static int __write_console(struct xencons_info *xencons,
> > +             const char *data, int len)
> >  {
> > -     struct xencons_interface *intf = xencons_interface();
> >       XENCONS_RING_IDX cons, prod;
> > +     struct xencons_interface *intf = xencons->intf;
> >       int sent = 0;
> >
> >       cons = intf->out_cons;
> > @@ -85,13 +108,16 @@ static int __write_console(const char *data, int len)
> >       intf->out_prod = prod;
> >
> >       if (sent)
> > -             notify_daemon();
> > +             notify_daemon(xencons);
> >       return sent;
> >  }
> >
> >  static int domU_write_console(uint32_t vtermno, const char *data, int len)
> >  {
> >       int ret = len;
> > +     struct xencons_info *cons = vtermno_to_xencons(vtermno);
> > +     if (cons == NULL)
> > +             return -EINVAL;
> >
> >       /*
> >        * Make sure the whole buffer is emitted, polling if
> > @@ -100,7 +126,7 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
> >        * kernel is crippled.
> >        */
> >       while (len) {
> > -             int sent = __write_console(data, len);
> > +             int sent = __write_console(cons, data, len);
> >
> >               data += sent;
> >               len -= sent;
> > @@ -114,9 +140,13 @@ static int domU_write_console(uint32_t vtermno, const char *data, int len)
> >
> >  static int domU_read_console(uint32_t vtermno, char *buf, int len)
> >  {
> > -     struct xencons_interface *intf = xencons_interface();
> > +     struct xencons_interface *intf;
> >       XENCONS_RING_IDX cons, prod;
> >       int recv = 0;
> > +     struct xencons_info *xencons = vtermno_to_xencons(vtermno);
> > +     if (xencons == NULL)
> > +             return -EINVAL;
> > +     intf = xencons->intf;
> >
> >       cons = intf->in_cons;
> >       prod = intf->in_prod;
> > @@ -129,7 +159,7 @@ static int domU_read_console(uint32_t vtermno, char *buf, int len)
> >       mb();                   /* read ring before consuming */
> >       intf->in_cons = cons;
> >
> > -     notify_daemon();
> > +     notify_daemon(xencons);
> >       return recv;
> >  }
> >
> > @@ -172,33 +202,109 @@ static int xen_hvm_console_init(void)
> >       int r;
> >       uint64_t v = 0;
> >       unsigned long mfn;
> > +     struct xencons_info *info;
> >
> >       if (!xen_hvm_domain())
> >               return -ENODEV;
> >
> > -     if (xencons_if != NULL)
> > -             return -EBUSY;
> > +     info = vtermno_to_xencons(HVC_COOKIE);
> > +     if (!info) {
> > +             info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
> > +             if (!info)
> > +                     return -ENOMEM;
> > +     }
> > +
> > +     /* already configured */
> > +     if (info->intf != NULL)
> > +             return 0;
> >
> >       r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
> > -     if (r < 0)
> > +     if (r < 0) {
> > +             kfree(info);
> >               return -ENODEV;
> > -     console_evtchn = v;
> > +     }
> > +     info->evtchn = v;
> >       hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> > -     if (r < 0)
> > +     if (r < 0) {
> > +             kfree(info);
> >               return -ENODEV;
> > +     }
> >       mfn = v;
> > -     xencons_if = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
> > -     if (xencons_if == NULL)
> > +     info->intf = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);
> > +     if (info->intf == NULL) {
> > +             kfree(info);
> > +             return -ENODEV;
> > +     }
> > +     info->vtermno = HVC_COOKIE;
> > +
> > +     spin_lock(&xencons_lock);
> > +     list_add_tail(&info->list, &xenconsoles);
> > +     spin_unlock(&xencons_lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static int xen_pv_console_init(void)
> > +{
> > +     struct xencons_info *info;
> > +
> > +     if (!xen_pv_domain())
> >               return -ENODEV;
> >
> > +     if (!xen_start_info->console.domU.evtchn)
> > +             return -ENODEV;
> > +
> > +     info = vtermno_to_xencons(HVC_COOKIE);
> > +     if (!info) {
> > +             info = kmalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
> 
> Ugh. Use kzalloc here. Especially as you are testing it below (and
> kmalloc with certain CONFIG_DEBUG.. can make the the returned memory
> have bogus data.

OK

Re: [Xen-devel] [PATCH 2/2] hvc_xen: implement multiconsole support

[Konrad Rzeszutek Wilk]

On Fri, Jan 27, 2012 at 05:16:45PM +0000, Stefano Stabellini wrote:
> On Fri, 27 Jan 2012, Konrad Rzeszutek Wilk wrote:
> > On Thu, Jan 26, 2012 at 12:43:27PM +0000, Stefano Stabellini wrote:
> > > This patch implements support for multiple consoles:
> > > consoles other than the first one are setup using the traditional xenbus
> > > and grant-table based mechanism.
> > > We use a list to keep track of the allocated consoles, we don't
> > > expect too many of them anyway.
> > >
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > ---
> > >  drivers/tty/hvc/hvc_xen.c |  439 +++++++++++++++++++++++++++++++++++++++------
> > >  1 files changed, 383 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> > > index dd6641f..97732fb 100644
> > > --- a/drivers/tty/hvc/hvc_xen.c
> > > +++ b/drivers/tty/hvc/hvc_xen.c
> > > @@ -23,6 +23,7 @@
> > >  #include <linux/err.h>
> > >  #include <linux/init.h>
> > >  #include <linux/types.h>
> > > +#include <linux/list.h>
> > >
> > >  #include <asm/io.h>
> > >  #include <asm/xen/hypervisor.h>
> > > @@ -30,47 +31,69 @@
> > >  #include <xen/xen.h>
> > >  #include <xen/interface/xen.h>
> > >  #include <xen/hvm.h>
> > > +#include <xen/grant_table.h>
> > >  #include <xen/page.h>
> > >  #include <xen/events.h>
> > >  #include <xen/interface/io/console.h>
> > >  #include <xen/hvc-console.h>
> > > +#include <xen/xenbus.h>
> > >
> > >  #include "hvc_console.h"
> > >
> > >  #define HVC_COOKIE   0x58656e /* "Xen" in hex */
> > >
> > > -static struct hvc_struct *hvc;
> > > -static int xencons_irq;
> > > +struct xencons_info {
> > > +     struct list_head list;
> > > +     struct xenbus_device *xbdev;
> > > +     struct xencons_interface *intf;
> > > +     unsigned int evtchn;
> > > +     struct hvc_struct *hvc;
> > > +     int irq;
> > > +     int vtermno;
> > > +     grant_ref_t gntref;
> > > +};
> > > +
> > > +static LIST_HEAD(xenconsoles);
> > > +static DEFINE_SPINLOCK(xencons_lock);
> > > +static struct xenbus_driver xencons_driver;
> > >
> > >  /* ------------------------------------------------------------------ */
> > >
> > > -static unsigned long console_pfn = ~0ul;
> > > -static unsigned int console_evtchn = ~0ul;
> > > -static struct xencons_interface *xencons_if = NULL;
> > > +static struct xencons_info *vtermno_to_xencons(int vtermno)
> > > +{
> > > +     struct xencons_info *entry, *ret = NULL;
> > > +
> > > +     if (list_empty(&xenconsoles))
> > > +                     return NULL;
> > >
> > > -static inline struct xencons_interface *xencons_interface(void)
> > > +     spin_lock(&xencons_lock);
> > 
> > This spinlock gets hit everytime something is typed or written on the
> > console right? Isn't there an spinlock already in the hvc driver...
> > 
> > Or are we protected by the vtermnos being checked for -1?
> 
> I think you are right: the spinlock is there to protect access to the
> list, so we can change list_for_each_entry to list_for_each_entry_safe
> in vtermno_to_xencons and we solve the problem. All the other spinlock
> acquisitions are done at console creation/destruction.

Right. So I think this will work as long as you mkae the call
to hvc_remove _before_ your remove entries from the xenconsoles.

That way hvc_remove will set vtermnos[x] to -1 and inhibit
any callers from calling into us.

This means you will need to redo the xencons_remove and xencons_free
a bit.. Hmm, there looks to be dead-lock there - let me send the
details.

Re: [Xen-devel] [PATCH 2/2] hvc_xen: implement multiconsole support

[Konrad Rzeszutek Wilk]

> +static int xencons_remove(struct xenbus_device *dev)
> +{
> +	struct xencons_info *info = dev_get_drvdata(&dev->dev);
> +
> +	spin_lock(&xencons_lock);
> +	list_del(&info->list);
> +	spin_unlock(&xencons_lock);
> +	xencons_free(info);
>  	return 0;
>  }
.. snip..
>  static void __exit xen_hvc_fini(void)
>  {
> -	if (hvc)
> -		hvc_remove(hvc);
> +	struct xencons_info *entry, *next;
> +
> +	if (list_empty(&xenconsoles))
> +			return;
> +
> +	spin_lock(&xencons_lock);

You take a lock.
> +	list_for_each_entry_safe(entry, next, &xenconsoles, list) {
> +		list_del(&entry->list);
> +		if (entry->xbdev)
> +			xencons_remove(entry->xbdev);

And then call xencons_remove which also takes the same lock.
> +		else {
> +			if (entry->irq > 0)
> +				unbind_from_irqhandler(entry->irq, NULL);
> +			if (entry->hvc);
> +				hvc_remove(entry->hvc);
> +			kfree(entry);
> +		}
> +	}
> +	spin_unlock(&xencons_lock);