Stefano Stabellini
2012-Jan-27 18:30 UTC
[PATCH v2 0/2] Xen (PV and HVM) multiple PV consoles
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. Changes in v2: - nicer safety check in patch #1; - kzalloc instead of kmalloc; - removed the spin_lock in vtermno_to_xencons, use list_for_each_entry_safe. 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 | 455 ++++++++++++++++++++++++++++++++---- include/xen/interface/hvm/params.h | 6 +- 2 files changed, 420 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-2 Cheers, Stefano
Stefano Stabellini
2012-Jan-27 18:31 UTC
[PATCH v2 1/2] hvc_xen: support PV on HVM consoles
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- drivers/tty/hvc/hvc_xen.c | 84 +++++++++++++++++++++++++++++------- include/xen/interface/hvm/params.h | 6 ++- 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c index 52fdf60..d5000aa 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,63 @@ 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_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()) { + if (!xen_start_info->console.domU.evtchn) + return -ENODEV; + 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 +231,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 +248,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 +279,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
Stefano Stabellini
2012-Jan-27 18:31 UTC
[PATCH v2 2/2] hvc_xen: implement multiconsole support
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, 381 insertions(+), 58 deletions(-) diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c index d5000aa..8209293 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,67 @@ #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, *n, *ret = NULL; + + if (list_empty(&xenconsoles)) + return NULL; -static inline struct xencons_interface *xencons_interface(void) + list_for_each_entry_safe(entry, n, &xenconsoles, list) { + if (entry->vtermno == vtermno) { + ret = entry; + break; + } + } + + 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 +106,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 +124,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 +138,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 +157,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,78 +200,360 @@ 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 = kzalloc(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 = kzalloc(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 = kzalloc(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; 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()) { - if (!xen_start_info->console.domU.evtchn) - return -ENODEV; - 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 = kzalloc(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) @@ -256,18 +566,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
Konrad Rzeszutek Wilk
2012-Jan-27 18:45 UTC
Re: [PATCH v2 2/2] hvc_xen: implement multiconsole support
> +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); > +I would say put xencons_disconnect_backend(info) here, that way it calls hvc_remove first, and then this would delete an "not-called" anymore structure.> + spin_lock(&xencons_lock); > + list_del(&info->list); > + spin_unlock(&xencons_lock); > + xencons_free(info);> return 0; > } >.. snip..> + > +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);Take that spin-lock out.> + list_for_each_entry_safe(entry, next, &xenconsoles, list) { > + list_del(&entry->list); > + if (entry->xbdev) > + xencons_remove(entry->xbdev);This guy deletes itself from the list..> + else { > + if (entry->irq > 0) > + unbind_from_irqhandler(entry->irq, NULL); > + if (entry->hvc); > + hvc_remove(entry->hvc); > + kfree(entry);..but this one does not. You could make xencons_remove just remove itself from the list and nothing else. Then rename it to ''xencons_remove_itself'' perhaps? After that, introduce a new function ''xencons_delete'' that would call xecons_disconnect_backend, xencons_remove_itself, and xencons_free?> + } > + } > + spin_unlock(&xencons_lock); > } > > static int xen_cons_init(void) > @@ -256,18 +566,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 = {This needs to be wrapped in the new macro that Jan prepared. DEFINE_XENBUS_DRIVER (73db144b58a32fc39733db6a7e1fe582072ad26a)> + .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
Konrad Rzeszutek Wilk
2012-Jan-27 18:45 UTC
Re: [PATCH v2 1/2] hvc_xen: support PV on HVM consoles
On Fri, Jan 27, 2012 at 06:31:36PM +0000, Stefano Stabellini wrote:> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>applied.> --- > drivers/tty/hvc/hvc_xen.c | 84 +++++++++++++++++++++++++++++------- > include/xen/interface/hvm/params.h | 6 ++- > 2 files changed, 73 insertions(+), 17 deletions(-) > > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c > index 52fdf60..d5000aa 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,63 @@ 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_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()) { > + if (!xen_start_info->console.domU.evtchn) > + return -ENODEV; > + 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 +231,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 +248,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 +279,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 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
Stefano Stabellini
2012-Jan-30 15:33 UTC
Re: [PATCH v2 2/2] hvc_xen: implement multiconsole support
On Fri, 27 Jan 2012, Konrad Rzeszutek Wilk wrote:> > +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); > > + > > I would say put > xencons_disconnect_backend(info) > > here, that way it calls hvc_remove first, and then this would > delete an "not-called" anymore structure. > > > + spin_lock(&xencons_lock); > > + list_del(&info->list); > > + spin_unlock(&xencons_lock); > > + xencons_free(info); > > > return 0; > > } > > > > .. snip.. > > + > > +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); > > Take that spin-lock out. > > > + list_for_each_entry_safe(entry, next, &xenconsoles, list) { > > + list_del(&entry->list); > > + if (entry->xbdev) > > + xencons_remove(entry->xbdev); > > This guy deletes itself from the list.. > > + else { > > + if (entry->irq > 0) > > + unbind_from_irqhandler(entry->irq, NULL); > > + if (entry->hvc); > > + hvc_remove(entry->hvc); > > + kfree(entry); > > ..but this one does not. You could make xencons_remove just remove > itself from the list and nothing else. Then rename it to ''xencons_remove_itself'' perhaps? > > After that, introduce a new function ''xencons_delete'' that would call > xecons_disconnect_backend, xencons_remove_itself, and xencons_free?Thanks for spotting the deadlock and the useful suggestions. I have reworked the shutdown path a bit, making sure that we don''t take any locks twice and that we call hvc_remove before list_del (see next version of the patch, to be posted soon).> > > > +static struct xenbus_driver xencons_driver = { > > This needs to be wrapped in the new macro that Jan prepared. > DEFINE_XENBUS_DRIVER (73db144b58a32fc39733db6a7e1fe582072ad26a)OK