| Message ID | 1456316590-20020-1-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/24/2016 07:23 AM, Stefano Stabellini wrote: > Introduce EARLYCON support in hvc_xen, useful for early debugging on arm > and arm64, where xen early_printk is not available. Differently from > xenboot_write_console on x86, we won't just return if !xen_pv_domain(), > because arm and arm64 guests are actually xen_hvm_domain() from linux > pov, and also because we want to capture all possible writes, including > the ones earlier than xen_early_init(). > > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > --- > drivers/tty/hvc/hvc_xen.c | 48 ++++++++++++++++++++++++++++++++++----------- > 1 file changed, 37 insertions(+), 11 deletions(-) > > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c > index fa816b7..34e8e9f 100644 > --- a/drivers/tty/hvc/hvc_xen.c > +++ b/drivers/tty/hvc/hvc_xen.c > @@ -25,6 +25,7 @@ > #include <linux/init.h> > #include <linux/types.h> > #include <linux/list.h> > +#include <linux/serial_core.h> > > #include <asm/io.h> > #include <asm/xen/hypervisor.h> > @@ -597,21 +598,11 @@ static int xen_cons_init(void) > } > console_initcall(xen_cons_init); > > -#ifdef CONFIG_EARLY_PRINTK > -static void xenboot_write_console(struct console *console, const char *string, > - unsigned len) > +static void domU_write_early_console(const char *string, unsigned len) > { > unsigned int linelen, off = 0; > const char *pos; > > - if (!xen_pv_domain()) > - return; > - > - dom0_write_console(0, string, len); > - > - if (xen_initial_domain()) > - return; > - > domU_write_console(0, "(early) ", 8); > while (off < len && NULL != (pos = strchr(string+off, '\n'))) { > linelen = pos-string+off; > @@ -625,6 +616,21 @@ static void xenboot_write_console(struct console *console, const char *string, > domU_write_console(0, string+off, len-off); > } > > +#ifdef CONFIG_EARLY_PRINTK > +static void xenboot_write_console(struct console *console, const char *string, > + unsigned len) > +{ > + if (!xen_pv_domain()) > + return; This, BTW, will break PVHv2 code (which is not there yet). Your patch is not making the code any different from what it is now but I wonder whether xenboot_write_console() can be made to work for HVM guests at this time since you making changes there anyway. Or we can delay it until HVMlite/PVHv2 patches make it in. > + > + dom0_write_console(0, string, len); > + > + if (xen_initial_domain()) > + return; > + > + domU_write_early_console(string, len); > +} > + > struct console xenboot_console = { > .name = "xenboot", > .write = xenboot_write_console, > @@ -664,3 +670,23 @@ void xen_raw_printk(const char *fmt, ...) > > xen_raw_console_write(buf); > } > + > +static void xenboot_earlycon_write(struct console *console, > + const char *string, > + unsigned len) > +{ > + dom0_write_console(0, string, len); > + > + if (xen_initial_domain()) > + return; > + > + domU_write_early_console(string, len); > +} xenboot_earlycon_write() and xenboot_write_console() share most of the code, can it be factored out? -boris > + > +static int __init xenboot_earlycon_setup(struct earlycon_device *device, > + const char *opt) > +{ > + device->con->write = xenboot_earlycon_write; > + return 0; > +} > +EARLYCON_DECLARE(xenboot, xenboot_earlycon_setup);
On Wed, 24 Feb 2016, Boris Ostrovsky wrote: > On 02/24/2016 07:23 AM, Stefano Stabellini wrote: > > Introduce EARLYCON support in hvc_xen, useful for early debugging on arm > > and arm64, where xen early_printk is not available. Differently from > > xenboot_write_console on x86, we won't just return if !xen_pv_domain(), > > because arm and arm64 guests are actually xen_hvm_domain() from linux > > pov, and also because we want to capture all possible writes, including > > the ones earlier than xen_early_init(). > > > > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > > --- > > drivers/tty/hvc/hvc_xen.c | 48 > > ++++++++++++++++++++++++++++++++++----------- > > 1 file changed, 37 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c > > index fa816b7..34e8e9f 100644 > > --- a/drivers/tty/hvc/hvc_xen.c > > +++ b/drivers/tty/hvc/hvc_xen.c > > @@ -25,6 +25,7 @@ > > #include <linux/init.h> > > #include <linux/types.h> > > #include <linux/list.h> > > +#include <linux/serial_core.h> > > #include <asm/io.h> > > #include <asm/xen/hypervisor.h> > > @@ -597,21 +598,11 @@ static int xen_cons_init(void) > > } > > console_initcall(xen_cons_init); > > -#ifdef CONFIG_EARLY_PRINTK > > -static void xenboot_write_console(struct console *console, const char > > *string, > > - unsigned len) > > +static void domU_write_early_console(const char *string, unsigned len) > > { > > unsigned int linelen, off = 0; > > const char *pos; > > - if (!xen_pv_domain()) > > - return; > > - > > - dom0_write_console(0, string, len); > > - > > - if (xen_initial_domain()) > > - return; > > - > > domU_write_console(0, "(early) ", 8); > > while (off < len && NULL != (pos = strchr(string+off, '\n'))) { > > linelen = pos-string+off; > > @@ -625,6 +616,21 @@ static void xenboot_write_console(struct console > > *console, const char *string, > > domU_write_console(0, string+off, len-off); > > } > > +#ifdef CONFIG_EARLY_PRINTK > > +static void xenboot_write_console(struct console *console, const char > > *string, > > + unsigned len) > > +{ > > + if (!xen_pv_domain()) > > + return; > > This, BTW, will break PVHv2 code (which is not there yet). Your patch is not > making the code any different from what it is now but I wonder whether > xenboot_write_console() can be made to work for HVM guests at this time since > you making changes there anyway. > > Or we can delay it until HVMlite/PVHv2 patches make it in. I don't think it can be made to work with HVM guests unfortunately, because in the HVM case Xen support is going to be discovered only at some later point. We have the same problem on ARM, where all guests are xen_hvm_domain(). In that case I solved the problem by calling dom0_write_console unconditionally from xenboot_earlycon_write, see below. I could do the same here by dropping the if (!xen_pv_domain()) check above, but then if somebody specifies earlyprintk=xenboot on a non-Xen environment, I expect Linux would crash. > > + > > + dom0_write_console(0, string, len); > > + > > + if (xen_initial_domain()) > > + return; > > + > > + domU_write_early_console(string, len); > > +} > > + > > struct console xenboot_console = { > > .name = "xenboot", > > .write = xenboot_write_console, > > @@ -664,3 +670,23 @@ void xen_raw_printk(const char *fmt, ...) > > xen_raw_console_write(buf); > > } > > + > > +static void xenboot_earlycon_write(struct console *console, > > + const char *string, > > + unsigned len) > > +{ > > + dom0_write_console(0, string, len); > > + > > + if (xen_initial_domain()) > > + return; > > + > > + domU_write_early_console(string, len); > > +} My explanation above made me realize that actually domU_write_early_console can never be called here, so I'll drop it in the next version. > xenboot_earlycon_write() and xenboot_write_console() share most of the code, > can it be factored out? No, they are actually different. I don't think it would be better to introduce #ifdef CONFIG_X86 in this file. > > + > > +static int __init xenboot_earlycon_setup(struct earlycon_device *device, > > + const char *opt) > > +{ > > + device->con->write = xenboot_earlycon_write; > > + return 0; > > +} > > +EARLYCON_DECLARE(xenboot, xenboot_earlycon_setup); >
> I could do the same here by dropping the if (!xen_pv_domain()) check > above, but then if somebody specifies earlyprintk=xenboot on a non-Xen > environment, I expect Linux would crash. Nah, you made it "Work" with: commit eb5ef07151ba3c3cb4bcef0c8f146ff1115eaa55 Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Date: Fri Jan 27 18:31:36 2012 +0000 hvc_xen: support PV on HVM consoles But this patch: commit 04b772d2b819f0dda2163e3193fa7cd447a6245c xen/hvc: If we use xen_raw_printk let it also work on HVM guests. The xen_raw_printk works great for debugging purposes. We use for PV guests and we can also use it for HVM guests. However, for HVM guests we have a fallback of using the 0xe9 port in case the hypervisor does not support an HVM guest of using the console_io hypercall. As such lets use 0xe9 during early bootup, and once the hyper-page is setup and if the console_io hypercall is supported - use that. Otherwise we will fallback to using the 0xe9 after early bootup. We also alter the return value for dom0_write_console to return an error value instead of zero. The HVC API has been supporting returning error values for quite some time. P.S. To use (and to see the output in the Xen ring buffer) one has to build the hypervisor with 'debug=y'. Should make it possible for HVM guests to actually work with HVM x86 guests if tweaked.
On 24/02/16 17:18, Konrad Rzeszutek Wilk wrote: >> I could do the same here by dropping the if (!xen_pv_domain()) check >> above, but then if somebody specifies earlyprintk=xenboot on a non-Xen >> environment, I expect Linux would crash. > Nah, you made it "Work" with: > commit eb5ef07151ba3c3cb4bcef0c8f146ff1115eaa55 > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Date: Fri Jan 27 18:31:36 2012 +0000 > > hvc_xen: support PV on HVM consoles > > But this patch: > > commit 04b772d2b819f0dda2163e3193fa7cd447a6245c > > xen/hvc: If we use xen_raw_printk let it also work on HVM guests. > > The xen_raw_printk works great for debugging purposes. We use > for PV guests and we can also use it for HVM guests. > > However, for HVM guests we have a fallback of using the 0xe9 > port in case the hypervisor does not support an HVM guest of > using the console_io hypercall. As such lets use 0xe9 during > early bootup, and once the hyper-page is setup and if the > console_io hypercall is supported - use that. Otherwise we > will fallback to using the 0xe9 after early bootup. > > We also alter the return value for dom0_write_console to return > an error value instead of zero. The HVC API has been supporting > returning error values for quite some time. > > P.S. > To use (and to see the output in the Xen ring buffer) one has to build > the hypervisor with 'debug=y'. > > Should make it possible for HVM guests to actually work with HVM x86 guests > if tweaked. /me looks +outb_print: + for (i = 0; i < len; i++) + outb(str[i], 0xe9); +#endif You already have the length to hand. Use outsb instead, for substantially fewer vmexits. ~Andrew
On Wed, 24 Feb 2016, Konrad Rzeszutek Wilk wrote: > > I could do the same here by dropping the if (!xen_pv_domain()) check > > above, but then if somebody specifies earlyprintk=xenboot on a non-Xen > > environment, I expect Linux would crash. > > Nah, you made it "Work" with: > commit eb5ef07151ba3c3cb4bcef0c8f146ff1115eaa55 > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Date: Fri Jan 27 18:31:36 2012 +0000 > > hvc_xen: support PV on HVM consoles > > But this patch: > > commit 04b772d2b819f0dda2163e3193fa7cd447a6245c > > xen/hvc: If we use xen_raw_printk let it also work on HVM guests. > > The xen_raw_printk works great for debugging purposes. We use > for PV guests and we can also use it for HVM guests. > > However, for HVM guests we have a fallback of using the 0xe9 > port in case the hypervisor does not support an HVM guest of > using the console_io hypercall. As such lets use 0xe9 during > early bootup, and once the hyper-page is setup and if the > console_io hypercall is supported - use that. Otherwise we > will fallback to using the 0xe9 after early bootup. > > We also alter the return value for dom0_write_console to return > an error value instead of zero. The HVC API has been supporting > returning error values for quite some time. > > P.S. > To use (and to see the output in the Xen ring buffer) one has to build > the hypervisor with 'debug=y'. > > Should make it possible for HVM guests to actually work with HVM x86 guests > if tweaked. That only makes xen_raw_printk and xen_raw_console_write work with HVM x86 guests, not the generic early_printk calls, usually enabled in PV guests with "earlyprintk=xen" on the kernel command line. But the xen_cpuid_base() check and outbs could be used in xenboot_write_console too to make early_printk work. I'll add a patch for that.
On Wed, 24 Feb 2016, Andrew Cooper wrote: > On 24/02/16 17:18, Konrad Rzeszutek Wilk wrote: > >> I could do the same here by dropping the if (!xen_pv_domain()) check > >> above, but then if somebody specifies earlyprintk=xenboot on a non-Xen > >> environment, I expect Linux would crash. > > Nah, you made it "Work" with: > > commit eb5ef07151ba3c3cb4bcef0c8f146ff1115eaa55 > > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Date: Fri Jan 27 18:31:36 2012 +0000 > > > > hvc_xen: support PV on HVM consoles > > > > But this patch: > > > > commit 04b772d2b819f0dda2163e3193fa7cd447a6245c > > > > xen/hvc: If we use xen_raw_printk let it also work on HVM guests. > > > > The xen_raw_printk works great for debugging purposes. We use > > for PV guests and we can also use it for HVM guests. > > > > However, for HVM guests we have a fallback of using the 0xe9 > > port in case the hypervisor does not support an HVM guest of > > using the console_io hypercall. As such lets use 0xe9 during > > early bootup, and once the hyper-page is setup and if the > > console_io hypercall is supported - use that. Otherwise we > > will fallback to using the 0xe9 after early bootup. > > > > We also alter the return value for dom0_write_console to return > > an error value instead of zero. The HVC API has been supporting > > returning error values for quite some time. > > > > P.S. > > To use (and to see the output in the Xen ring buffer) one has to build > > the hypervisor with 'debug=y'. > > > > Should make it possible for HVM guests to actually work with HVM x86 guests > > if tweaked. > > /me looks > > +outb_print: > + for (i = 0; i < len; i++) > + outb(str[i], 0xe9); > +#endif > > You already have the length to hand. Use outsb instead, for > substantially fewer vmexits. I'll take the opportunity to make that change as well
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c index fa816b7..34e8e9f 100644 --- a/drivers/tty/hvc/hvc_xen.c +++ b/drivers/tty/hvc/hvc_xen.c @@ -25,6 +25,7 @@ #include <linux/init.h> #include <linux/types.h> #include <linux/list.h> +#include <linux/serial_core.h> #include <asm/io.h> #include <asm/xen/hypervisor.h> @@ -597,21 +598,11 @@ static int xen_cons_init(void) } console_initcall(xen_cons_init); -#ifdef CONFIG_EARLY_PRINTK -static void xenboot_write_console(struct console *console, const char *string, - unsigned len) +static void domU_write_early_console(const char *string, unsigned len) { unsigned int linelen, off = 0; const char *pos; - if (!xen_pv_domain()) - return; - - dom0_write_console(0, string, len); - - if (xen_initial_domain()) - return; - domU_write_console(0, "(early) ", 8); while (off < len && NULL != (pos = strchr(string+off, '\n'))) { linelen = pos-string+off; @@ -625,6 +616,21 @@ static void xenboot_write_console(struct console *console, const char *string, domU_write_console(0, string+off, len-off); } +#ifdef CONFIG_EARLY_PRINTK +static void xenboot_write_console(struct console *console, const char *string, + unsigned len) +{ + if (!xen_pv_domain()) + return; + + dom0_write_console(0, string, len); + + if (xen_initial_domain()) + return; + + domU_write_early_console(string, len); +} + struct console xenboot_console = { .name = "xenboot", .write = xenboot_write_console, @@ -664,3 +670,23 @@ void xen_raw_printk(const char *fmt, ...) xen_raw_console_write(buf); } + +static void xenboot_earlycon_write(struct console *console, + const char *string, + unsigned len) +{ + dom0_write_console(0, string, len); + + if (xen_initial_domain()) + return; + + domU_write_early_console(string, len); +} + +static int __init xenboot_earlycon_setup(struct earlycon_device *device, + const char *opt) +{ + device->con->write = xenboot_earlycon_write; + return 0; +} +EARLYCON_DECLARE(xenboot, xenboot_earlycon_setup);
Introduce EARLYCON support in hvc_xen, useful for early debugging on arm and arm64, where xen early_printk is not available. Differently from xenboot_write_console on x86, we won't just return if !xen_pv_domain(), because arm and arm64 guests are actually xen_hvm_domain() from linux pov, and also because we want to capture all possible writes, including the ones earlier than xen_early_init(). Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- drivers/tty/hvc/hvc_xen.c | 48 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 11 deletions(-)