Konrad Rzeszutek Wilk
2013-Aug-09 14:55 UTC
[PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <konrad.wilk@oracle.com>
---
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
Ian Campbell
2013-Aug-13 20:31 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <konrad.wilk@oracle.com> > --- > 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, ...)
Konrad Rzeszutek Wilk
2013-Aug-13 20:53 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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!
Ian Campbell
2013-Aug-13 20:59 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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.
Ian Campbell
2013-Aug-13 22:20 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <520A7145.5010306@tycho.nsa.gov> 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.
Konrad Rzeszutek Wilk
2013-Aug-14 00:10 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <520A7145.5010306@tycho.nsa.gov> 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. >
Ian Campbell
2013-Aug-14 07:15 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <520A7145.5010306@tycho.nsa.gov> 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.
Konrad Rzeszutek Wilk
2013-Aug-14 12:46 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <520A7145.5010306@tycho.nsa.gov> 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. >
Ian Campbell
2013-Aug-15 14:40 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <520A7145.5010306@tycho.nsa.gov> 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.
Konrad Rzeszutek Wilk
2013-Aug-20 19:35 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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 <520A7145.5010306@tycho.nsa.gov> 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 <konrad.wilk@oracle.com> 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 <konrad.wilk@oracle.com> --- 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. >
Ian Campbell
2013-Aug-21 09:15 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
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, ...)
Konrad Rzeszutek Wilk
2013-Aug-21 10:44 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
Ian Campbell <ian.campbell@citrix.com> 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.
Ian Campbell
2013-Aug-22 07:15 UTC
Re: [PATCH] xen/hvc: If we use xen_raw_printk let it also work on HVM guests.
On Wed, 2013-08-21 at 06:44 -0400, Konrad Rzeszutek Wilk wrote:> Ian Campbell <ian.campbell@citrix.com> 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.