The xen_raw_printk works great for debugging purposes and for
it print anything the Xen hypervisor has to be built with 'debug=y'.
As such there is no difference between a PV or an PVHVM guest
using the hypercall, so lets use it.
Lastly if the hyper-page is not setup yet (for example during
early HVM boot), then use the 0xe9 port if it has detected
that it is running under an Xen hypervisor.
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
drivers/tty/hvc/hvc_xen.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 682210d..69454a0 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -641,7 +641,17 @@ struct console xenboot_console = {
void xen_raw_console_write(const char *str)
{
- dom0_write_console(0, str, strlen(str));
+ if (!xen_domain())
+ return;
+
+ if (xen_pv_domain())
+ dom0_write_console(0, str, strlen(str));
+ else if (xen_hvm_domain() || xen_cpuid_base()) {
+ /* The hyperpage has not been setup yet. */
+ int i, len = strlen(str);
+ for (i = 0; i < len; i++)
+ outb(str[i], 0xe9);
+ }
}
void xen_raw_printk(const char *fmt, ...)
--
1.7.7.6
On Fri, 2013-08-09 at 10:55 -0400, Konrad Rzeszutek Wilk wrote:
> The xen_raw_printk works great for debugging purposes and for
> it print anything the Xen hypervisor has to be built with 'debug=y'.
>
> As such there is no difference between a PV or an PVHVM guest
> using the hypercall, so lets use it.
>
> Lastly if the hyper-page is not setup yet (for example during
> early HVM boot), then use the 0xe9 port if it has detected
> that it is running under an Xen hypervisor.
Does this really do what you say?
I think xen_pv_domain returns false for a PVHVM guest, meaning that we
only use the hypercall for proper PV guests and for PVHVM we use port
0xe9 until the hypercall page is setup at which point we silently
discard any attempt to print via this mechanism.
or am I reading it wrong?
>
>
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
> ---
> drivers/tty/hvc/hvc_xen.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 682210d..69454a0 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -641,7 +641,17 @@ struct console xenboot_console = {
>
> void xen_raw_console_write(const char *str)
> {
> - dom0_write_console(0, str, strlen(str));
> + if (!xen_domain())
> + return;
> +
> + if (xen_pv_domain())
> + dom0_write_console(0, str, strlen(str));
> + else if (xen_hvm_domain() || xen_cpuid_base()) {
> + /* The hyperpage has not been setup yet. */
> + int i, len = strlen(str);
> + for (i = 0; i < len; i++)
> + outb(str[i], 0xe9);
> + }
> }
>
> void xen_raw_printk(const char *fmt, ...)
On Tue, Aug 13, 2013 at 09:31:44PM +0100, Ian Campbell wrote:
> On Fri, 2013-08-09 at 10:55 -0400, Konrad Rzeszutek Wilk wrote:
> > The xen_raw_printk works great for debugging purposes and for
> > it print anything the Xen hypervisor has to be built with 'debug=y'.
> >
> > As such there is no difference between a PV or an PVHVM guest
> > using the hypercall, so lets use it.
> >
> > Lastly if the hyper-page is not setup yet (for example during
> > early HVM boot), then use the 0xe9 port if it has detected
> > that it is running under an Xen hypervisor.
>
> Does this really do what you say?
>
> I think xen_pv_domain returns false for a PVHVM guest, meaning that we
> only use the hypercall for proper PV guests and for PVHVM we use port
> 0xe9 until the hypercall page is setup at which point we silently
> discard any attempt to print via this mechanism.
>
'via this mechanism' refers to 0xe9 or dom0_write_console?
> or am I reading it wrong?
The code did have a bug as it would on HVM (after the hyper-page
was setup) still use 0xe9 instead of the dom0_write_console.
What it should be is:
> >
> > void xen_raw_console_write(const char *str)
> > {
> > - dom0_write_console(0, str, strlen(str));
> > + if (!xen_domain())
> > + return;
> > +
> > + if (xen_pv_domain())
xen_domain()
> > + dom0_write_console(0, str, strlen(str));
> > + else if (xen_hvm_domain() || xen_cpuid_base()) {
else if (xen_cpuid_base()) {
> > + /* The hyperpage has not been setup yet. */
> > + int i, len = strlen(str);
> > + for (i = 0; i < len; i++)
> > + outb(str[i], 0xe9);
> > + }
> > }
And then that should adhere to what I wrote up.
Thanks for pointing this out!
On Tue, 2013-08-13 at 16:53 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Aug 13, 2013 at 09:31:44PM +0100, Ian Campbell wrote:
> > On Fri, 2013-08-09 at 10:55 -0400, Konrad Rzeszutek Wilk wrote:
> > > The xen_raw_printk works great for debugging purposes and for
> > > it print anything the Xen hypervisor has to be built with 'debug=y'.
> > >
> > > As such there is no difference between a PV or an PVHVM guest
> > > using the hypercall, so lets use it.
> > >
> > > Lastly if the hyper-page is not setup yet (for example during
> > > early HVM boot), then use the 0xe9 port if it has detected
> > > that it is running under an Xen hypervisor.
> >
> > Does this really do what you say?
> >
> > I think xen_pv_domain returns false for a PVHVM guest, meaning that we
> > only use the hypercall for proper PV guests and for PVHVM we use port
> > 0xe9 until the hypercall page is setup at which point we silently
> > discard any attempt to print via this mechanism.
> >
>
> 'via this mechanism' refers to 0xe9 or dom0_write_console?
Neither, I meant the xen_raw_console_write mechanism.
>
> > or am I reading it wrong?
>
> The code did have a bug as it would on HVM (after the hyper-page
> was setup) still use 0xe9 instead of the dom0_write_console.
>
> What it should be is:
> > >
> > > void xen_raw_console_write(const char *str)
> > > {
> > > - dom0_write_console(0, str, strlen(str));
> > > + if (!xen_domain())
> > > + return;
> > > +
> > > + if (xen_pv_domain())
> xen_domain()
>
> > > + dom0_write_console(0, str, strlen(str));
> > > + else if (xen_hvm_domain() || xen_cpuid_base()) {
>
> else if (xen_cpuid_base()) {
>
> > > + /* The hyperpage has not been setup yet. */
> > > + int i, len = strlen(str);
> > > + for (i = 0; i < len; i++)
> > > + outb(str[i], 0xe9);
> > > + }
> > > }
>
> And then that should adhere to what I wrote up.
I think it does too.
> Thanks for pointing this out!
NP!
Ian.
On Tue, 2013-08-13 at 21:59 +0100, Ian Campbell wrote:
> > What it should be is:
> > > >
> > > > void xen_raw_console_write(const char *str)
> > > > {
> > > > - dom0_write_console(0, str, strlen(str));
> > > > + if (!xen_domain())
> > > > + return;
> > > > +
> > > > + if (xen_pv_domain())
> > xen_domain()
> >
> > > > + dom0_write_console(0, str, strlen(str));
> > > > + else if (xen_hvm_domain() || xen_cpuid_base()) {
> >
> > else if (xen_cpuid_base()) {
> >
> > > > + /* The hyperpage has not been setup yet. */
> > > > + int i, len = strlen(str);
> > > > + for (i = 0; i < len; i++)
> > > > + outb(str[i], 0xe9);
> > > > + }
> > > > }
> >
> > And then that should adhere to what I wrote up.
>
> I think it does too.
Except as Daniel notes in <[email protected]> for unrelated
reasons:
> HVM guests can still use the PV output - they just need to use the console
> write hypercall instead of the HVM I/O port. I would think that PVH guests
> would default to using the hypercall as it is more efficient (it takes a
> string rather than one character per write).
>
> Actually, checking... the console_io hypercall would need to be added to
> the hvm_hypercall{32,64}_table for an HVM guest to be able to use it; they
> currently must use the I/O port. I didn't check the PVH patches.
Or did you actually try this code and it worked?
Ian.
On Tue, Aug 13, 2013 at 11:20:05PM +0100, Ian Campbell wrote:
> On Tue, 2013-08-13 at 21:59 +0100, Ian Campbell wrote:
>
> > > What it should be is:
> > > > >
> > > > > void xen_raw_console_write(const char *str)
> > > > > {
> > > > > - dom0_write_console(0, str, strlen(str));
> > > > > + if (!xen_domain())
> > > > > + return;
> > > > > +
> > > > > + if (xen_pv_domain())
> > > xen_domain()
> > >
> > > > > + dom0_write_console(0, str, strlen(str));
> > > > > + else if (xen_hvm_domain() || xen_cpuid_base()) {
> > >
> > > else if (xen_cpuid_base()) {
> > >
> > > > > + /* The hyperpage has not been setup yet. */
> > > > > + int i, len = strlen(str);
> > > > > + for (i = 0; i < len; i++)
> > > > > + outb(str[i], 0xe9);
> > > > > + }
> > > > > }
> > >
> > > And then that should adhere to what I wrote up.
> >
> > I think it does too.
>
> Except as Daniel notes in <[email protected]> for unrelated
> reasons:
>
> > HVM guests can still use the PV output - they just need to use the console
> > write hypercall instead of the HVM I/O port. I would think that PVH guests
> > would default to using the hypercall as it is more efficient (it takes a
> > string rather than one character per write).
> >
> > Actually, checking... the console_io hypercall would need to be added to
> > the hvm_hypercall{32,64}_table for an HVM guest to be able to use it; they
> > currently must use the I/O port. I didn't check the PVH patches.
>
> Or did you actually try this code and it worked?
The one I typed up above - no. The one I had sent - yes.
But with that above mentioned comment from Daniel I think it is still
worth trying to do dom0_write_console and if the hypercall returns -ENOSYS
then fall back on 0xe9.
And lastly send an patch to make hypercall_io work under HVM.
>
> Ian.
>
On Tue, 2013-08-13 at 20:10 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Aug 13, 2013 at 11:20:05PM +0100, Ian Campbell wrote:
> > On Tue, 2013-08-13 at 21:59 +0100, Ian Campbell wrote:
> >
> > > > What it should be is:
> > > > > >
> > > > > > void xen_raw_console_write(const char *str)
> > > > > > {
> > > > > > - dom0_write_console(0, str, strlen(str));
> > > > > > + if (!xen_domain())
> > > > > > + return;
> > > > > > +
> > > > > > + if (xen_pv_domain())
> > > > xen_domain()
> > > >
> > > > > > + dom0_write_console(0, str, strlen(str));
> > > > > > + else if (xen_hvm_domain() || xen_cpuid_base()) {
> > > >
> > > > else if (xen_cpuid_base()) {
> > > >
> > > > > > + /* The hyperpage has not been setup yet. */
> > > > > > + int i, len = strlen(str);
> > > > > > + for (i = 0; i < len; i++)
> > > > > > + outb(str[i], 0xe9);
> > > > > > + }
> > > > > > }
> > > >
> > > > And then that should adhere to what I wrote up.
> > >
> > > I think it does too.
> >
> > Except as Daniel notes in <[email protected]> for unrelated
> > reasons:
> >
> > > HVM guests can still use the PV output - they just need to use the console
> > > write hypercall instead of the HVM I/O port. I would think that PVH guests
> > > would default to using the hypercall as it is more efficient (it takes a
> > > string rather than one character per write).
> > >
> > > Actually, checking... the console_io hypercall would need to be added to
> > > the hvm_hypercall{32,64}_table for an HVM guest to be able to use it; they
> > > currently must use the I/O port. I didn't check the PVH patches.
> >
> > Or did you actually try this code and it worked?
>
> The one I typed up above - no. The one I had sent - yes.
>
> But with that above mentioned comment from Daniel I think it is still
> worth trying to do dom0_write_console and if the hypercall returns -ENOSYS
> then fall back on 0xe9.
>
> And lastly send an patch to make hypercall_io work under HVM.
So you didn't try under PVHVM? That's what I was asking when I asked if
you tried it, since that is the point of this patch.
Ian.
Ian.
On Wed, Aug 14, 2013 at 08:15:56AM +0100, Ian Campbell wrote:
> On Tue, 2013-08-13 at 20:10 -0400, Konrad Rzeszutek Wilk wrote:
> > On Tue, Aug 13, 2013 at 11:20:05PM +0100, Ian Campbell wrote:
> > > On Tue, 2013-08-13 at 21:59 +0100, Ian Campbell wrote:
> > >
> > > > > What it should be is:
> > > > > > >
> > > > > > > void xen_raw_console_write(const char *str)
> > > > > > > {
> > > > > > > - dom0_write_console(0, str, strlen(str));
> > > > > > > + if (!xen_domain())
> > > > > > > + return;
> > > > > > > +
> > > > > > > + if (xen_pv_domain())
> > > > > xen_domain()
> > > > >
> > > > > > > + dom0_write_console(0, str, strlen(str));
> > > > > > > + else if (xen_hvm_domain() || xen_cpuid_base()) {
> > > > >
> > > > > else if (xen_cpuid_base()) {
> > > > >
> > > > > > > + /* The hyperpage has not been setup yet. */
> > > > > > > + int i, len = strlen(str);
> > > > > > > + for (i = 0; i < len; i++)
> > > > > > > + outb(str[i], 0xe9);
> > > > > > > + }
> > > > > > > }
> > > > >
> > > > > And then that should adhere to what I wrote up.
> > > >
> > > > I think it does too.
> > >
> > > Except as Daniel notes in <[email protected]> for unrelated
> > > reasons:
> > >
> > > > HVM guests can still use the PV output - they just need to use the console
> > > > write hypercall instead of the HVM I/O port. I would think that PVH guests
> > > > would default to using the hypercall as it is more efficient (it takes a
> > > > string rather than one character per write).
> > > >
> > > > Actually, checking... the console_io hypercall would need to be added to
> > > > the hvm_hypercall{32,64}_table for an HVM guest to be able to use it; they
> > > > currently must use the I/O port. I didn't check the PVH patches.
> > >
> > > Or did you actually try this code and it worked?
> >
> > The one I typed up above - no. The one I had sent - yes.
> >
> > But with that above mentioned comment from Daniel I think it is still
> > worth trying to do dom0_write_console and if the hypercall returns -ENOSYS
> > then fall back on 0xe9.
> >
> > And lastly send an patch to make hypercall_io work under HVM.
>
> So you didn't try under PVHVM? That's what I was asking when I asked if
> you tried it, since that is the point of this patch.
Not the one I typed up above. Just the one I sent. I have a habit of
sending patches that have been tested first - and I hadn't had the chance
yet to redo it the way above.
>
> Ian.
>
> Ian.
>
On Wed, 2013-08-14 at 08:46 -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Aug 14, 2013 at 08:15:56AM +0100, Ian Campbell wrote:
> > On Tue, 2013-08-13 at 20:10 -0400, Konrad Rzeszutek Wilk wrote:
> > > On Tue, Aug 13, 2013 at 11:20:05PM +0100, Ian Campbell wrote:
> > > > On Tue, 2013-08-13 at 21:59 +0100, Ian Campbell wrote:
> > > >
> > > > > > What it should be is:
> > > > > > > >
> > > > > > > > void xen_raw_console_write(const char *str)
> > > > > > > > {
> > > > > > > > - dom0_write_console(0, str, strlen(str));
> > > > > > > > + if (!xen_domain())
> > > > > > > > + return;
> > > > > > > > +
> > > > > > > > + if (xen_pv_domain())
> > > > > > xen_domain()
> > > > > >
> > > > > > > > + dom0_write_console(0, str, strlen(str));
> > > > > > > > + else if (xen_hvm_domain() || xen_cpuid_base()) {
> > > > > >
> > > > > > else if (xen_cpuid_base()) {
> > > > > >
> > > > > > > > + /* The hyperpage has not been setup yet. */
> > > > > > > > + int i, len = strlen(str);
> > > > > > > > + for (i = 0; i < len; i++)
> > > > > > > > + outb(str[i], 0xe9);
> > > > > > > > + }
> > > > > > > > }
> > > > > >
> > > > > > And then that should adhere to what I wrote up.
> > > > >
> > > > > I think it does too.
> > > >
> > > > Except as Daniel notes in <[email protected]> for unrelated
> > > > reasons:
> > > >
> > > > > HVM guests can still use the PV output - they just need to use the console
> > > > > write hypercall instead of the HVM I/O port. I would think that PVH guests
> > > > > would default to using the hypercall as it is more efficient (it takes a
> > > > > string rather than one character per write).
> > > > >
> > > > > Actually, checking... the console_io hypercall would need to be added to
> > > > > the hvm_hypercall{32,64}_table for an HVM guest to be able to use it; they
> > > > > currently must use the I/O port. I didn't check the PVH patches.
> > > >
> > > > Or did you actually try this code and it worked?
> > >
> > > The one I typed up above - no. The one I had sent - yes.
> > >
> > > But with that above mentioned comment from Daniel I think it is still
> > > worth trying to do dom0_write_console and if the hypercall returns -ENOSYS
> > > then fall back on 0xe9.
> > >
> > > And lastly send an patch to make hypercall_io work under HVM.
> >
> > So you didn't try under PVHVM? That's what I was asking when I asked if
> > you tried it, since that is the point of this patch.
>
> Not the one I typed up above. Just the one I sent.
And did it work? ecause if the console_io hypercall isn't wired up in
the hypervisor I can't see how it can have done...
Oh the original patch would buggily never have tried to use console_io,
and you probably didn't notice the lack of output after the hypercall
page was setup.
Ian.
On Thu, Aug 15, 2013 at 03:40:38PM +0100, Ian Campbell wrote:
> On Wed, 2013-08-14 at 08:46 -0400, Konrad Rzeszutek Wilk wrote:
> > On Wed, Aug 14, 2013 at 08:15:56AM +0100, Ian Campbell wrote:
> > > On Tue, 2013-08-13 at 20:10 -0400, Konrad Rzeszutek Wilk wrote:
> > > > On Tue, Aug 13, 2013 at 11:20:05PM +0100, Ian Campbell wrote:
> > > > > On Tue, 2013-08-13 at 21:59 +0100, Ian Campbell wrote:
> > > > >
> > > > > > > What it should be is:
> > > > > > > > >
> > > > > > > > > void xen_raw_console_write(const char *str)
> > > > > > > > > {
> > > > > > > > > - dom0_write_console(0, str, strlen(str));
> > > > > > > > > + if (!xen_domain())
> > > > > > > > > + return;
> > > > > > > > > +
> > > > > > > > > + if (xen_pv_domain())
> > > > > > > xen_domain()
> > > > > > >
> > > > > > > > > + dom0_write_console(0, str, strlen(str));
> > > > > > > > > + else if (xen_hvm_domain() || xen_cpuid_base()) {
> > > > > > >
> > > > > > > else if (xen_cpuid_base()) {
> > > > > > >
> > > > > > > > > + /* The hyperpage has not been setup yet. */
> > > > > > > > > + int i, len = strlen(str);
> > > > > > > > > + for (i = 0; i < len; i++)
> > > > > > > > > + outb(str[i], 0xe9);
> > > > > > > > > + }
> > > > > > > > > }
> > > > > > >
> > > > > > > And then that should adhere to what I wrote up.
> > > > > >
> > > > > > I think it does too.
> > > > >
> > > > > Except as Daniel notes in <[email protected]> for unrelated
> > > > > reasons:
> > > > >
> > > > > > HVM guests can still use the PV output - they just need to use the console
> > > > > > write hypercall instead of the HVM I/O port. I would think that PVH guests
> > > > > > would default to using the hypercall as it is more efficient (it takes a
> > > > > > string rather than one character per write).
> > > > > >
> > > > > > Actually, checking... the console_io hypercall would need to be added to
> > > > > > the hvm_hypercall{32,64}_table for an HVM guest to be able to use it; they
> > > > > > currently must use the I/O port. I didn't check the PVH patches.
> > > > >
> > > > > Or did you actually try this code and it worked?
> > > >
> > > > The one I typed up above - no. The one I had sent - yes.
> > > >
> > > > But with that above mentioned comment from Daniel I think it is still
> > > > worth trying to do dom0_write_console and if the hypercall returns -ENOSYS
> > > > then fall back on 0xe9.
> > > >
> > > > And lastly send an patch to make hypercall_io work under HVM.
> > >
> > > So you didn't try under PVHVM? That's what I was asking when I asked if
> > > you tried it, since that is the point of this patch.
> >
> > Not the one I typed up above. Just the one I sent.
>
> And did it work? ecause if the console_io hypercall isn't wired up in
> the hypervisor I can't see how it can have done...
>
> Oh the original patch would buggily never have tried to use console_io,
> and you probably didn't notice the lack of output after the hypercall
> page was setup.
<nods>
This is a patch below that works for me and also it work with hypervisors
that don't have console_io wired in.
Pls treat it as RFC
>From 5b5faae0038d48aa0169bab8c78d1b47c4f1ac64 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <[email protected]>
Date: Fri, 16 Aug 2013 15:32:07 -0400
Subject: [PATCH] xen/hvc: Print out to the serial console and pv console
irregardless if it is dom0, PV or HVM.
*TODO*: BLA BLA BLA
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
drivers/tty/hvc/hvc_xen.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 682210d..e47c6ae 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -40,6 +40,7 @@
#include <xen/hvc-console.h>
#include <xen/xenbus.h>
+#include <asm/xen/hypervisor.h>
#include "hvc_console.h"
#define HVC_COOKIE 0x58656e /* "Xen" in hex */
@@ -641,7 +642,19 @@ struct console xenboot_console = {
void xen_raw_console_write(const char *str)
{
- dom0_write_console(0, str, strlen(str));
+ ssize_t len = strlen(str);
+ int rc = 0;
+
+ if (xen_domain()) {
+ rc = dom0_write_console(0, str, len);
+ if (rc != len && xen_hvm_domain()) /* -ENOSYS */
+ goto outb_print;
+ } else if (xen_cpuid_base()) {
+ int i;
+outb_print:
+ for (i = 0; i < len; i++)
+ outb(str[i], 0xe9);
+ }
}
void xen_raw_printk(const char *fmt, ...)
--
1.7.7.6
> Ian.
>
On Tue, 2013-08-20 at 15:35 -0400, Konrad Rzeszutek Wilk wrote:
> void xen_raw_console_write(const char *str)
> {
> - dom0_write_console(0, str, strlen(str));
> + ssize_t len = strlen(str);
> + int rc = 0;
> +
> + if (xen_domain()) {
> + rc = dom0_write_console(0, str, len);
> + if (rc != len && xen_hvm_domain()) /* -ENOSYS */
If you want to catch ENOSYS then I suggest doing so explicitly, rather
that relying on len != -ENOSYS.
> + goto outb_print;
How about reversing this into
if (rc == len) return;
if (rc != -ENOSYS) panic(...) /* yes, this won't get far... *.
Then fall through to the following block as a plain if not an else if.
Maybe with a xen_hvm_domain && added.
That avoids the yucky goto I think.
> + } else if (xen_cpuid_base()) {
> + int i;
> +outb_print:
> + for (i = 0; i < len; i++)
> + outb(str[i], 0xe9);
> + }
> }
>
> void xen_raw_printk(const char *fmt, ...)
Ian Campbell <[email protected]> wrote:
>On Tue, 2013-08-20 at 15:35 -0400, Konrad Rzeszutek Wilk wrote:
>> void xen_raw_console_write(const char *str)
>> {
>> - dom0_write_console(0, str, strlen(str));
>> + ssize_t len = strlen(str);
>> + int rc = 0;
>> +
>> + if (xen_domain()) {
>> + rc = dom0_write_console(0, str, len);
>> + if (rc != len && xen_hvm_domain()) /* -ENOSYS */
>
>If you want to catch ENOSYS then I suggest doing so explicitly, rather
>that relying on len != -ENOSYS.
>
>> + goto outb_print;
>
>How about reversing this into
> if (rc == len) return;
> if (rc != -ENOSYS) panic(...) /* yes, this won't get far... *.
>
>Then fall through to the following block as a plain if not an else if.
>Maybe with a xen_hvm_domain && added.
>
>That avoids the yucky goto I think.
>
>> + } else if (xen_cpuid_base()) {
>> + int i;
>> +outb_print:
>> + for (i = 0; i < len; i++)
>> + outb(str[i], 0xe9);
>> + }
>> }
>>
>> void xen_raw_printk(const char *fmt, ...)
B/c the function dom0_write_console never returns negative values. Instead it converts it to a zero return value. Perhaps it can return negative values - will have to look.
On Wed, 2013-08-21 at 06:44 -0400, Konrad Rzeszutek Wilk wrote:
> Ian Campbell <[email protected]> wrote:
> >On Tue, 2013-08-20 at 15:35 -0400, Konrad Rzeszutek Wilk wrote:
> >> void xen_raw_console_write(const char *str)
> >> {
> >> - dom0_write_console(0, str, strlen(str));
> >> + ssize_t len = strlen(str);
> >> + int rc = 0;
> >> +
> >> + if (xen_domain()) {
> >> + rc = dom0_write_console(0, str, len);
> >> + if (rc != len && xen_hvm_domain()) /* -ENOSYS */
> >
> >If you want to catch ENOSYS then I suggest doing so explicitly, rather
> >that relying on len != -ENOSYS.
> >
> >> + goto outb_print;
> >
> >How about reversing this into
> > if (rc == len) return;
> > if (rc != -ENOSYS) panic(...) /* yes, this won't get far... *.
> >
> >Then fall through to the following block as a plain if not an else if.
> >Maybe with a xen_hvm_domain && added.
> >
> >That avoids the yucky goto I think.
> >
> >> + } else if (xen_cpuid_base()) {
> >> + int i;
> >> +outb_print:
> >> + for (i = 0; i < len; i++)
> >> + outb(str[i], 0xe9);
> >> + }
> >> }
> >>
> >> void xen_raw_printk(const char *fmt, ...)
>
> B/c the function dom0_write_console never returns negative values.
> Instead it converts it to a zero return value. Perhaps it can return
> negative values - will have to look.
At least the virtio_console version of the hv_ops.put_chars hook can
return -errno. Perhaps this has changed since it was implemented.
Ian.