[PATCH] xen/hvc-console: Make it work with HVM guests.

[PATCH] xen/hvc-console: Make it work with HVM guests.

[Konrad Rzeszutek Wilk]

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/tty/hvc/hvc_xen.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index e61c36c..513a79b 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -183,7 +183,7 @@ static int dom0_write_console(uint32_t vtermno, const char *str, int len)
 {
 	int rc = HYPERVISOR_console_io(CONSOLEIO_write, len, (char *)str);
 	if (rc < 0)
-		return 0;
+		return rc;
 
 	return len;
 }
@@ -641,7 +641,20 @@ 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()) {
+		dom0_write_console(0, str, len);
+		if (rc == -ENOSYS && xen_hvm_domain())
+			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

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Julien Grall]

On 09/27/2013 10:25 PM, Konrad Rzeszutek Wilk wrote:

> @@ -641,7 +641,20 @@ 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()) {
> +		dom0_write_console(0, str, len);
> +		if (rc == -ENOSYS && xen_hvm_domain())
> +			goto outb_print;
> +
> +	} else if (xen_cpuid_base()) {
> +		int i;
> +outb_print:
> +		for (i = 0; i < len; i++)
> +			outb(str[i], 0xe9);
> +	}
>   }

xen_cpuid_base and outb(0xe9) is x86 specific and won't compile on ARM.
You need to add ifdef around.

-- 
Julien Grall

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Konrad Rzeszutek Wilk]

On Fri, Sep 27, 2013 at 10:49:37PM +0100, Julien Grall wrote:
> On 09/27/2013 10:25 PM, Konrad Rzeszutek Wilk wrote:
> 
> >@@ -641,7 +641,20 @@ 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()) {
> >+		dom0_write_console(0, str, len);
> >+		if (rc == -ENOSYS && xen_hvm_domain())
> >+			goto outb_print;
> >+
> >+	} else if (xen_cpuid_base()) {
> >+		int i;
> >+outb_print:
> >+		for (i = 0; i < len; i++)
> >+			outb(str[i], 0xe9);
> >+	}
> >  }
> 
> xen_cpuid_base and outb(0xe9) is x86 specific and won't compile on ARM.

Odd, I see outb defined in arch/arm and arch/arm64 ?(arch/arm[|64]/include/asm.io.h)

You are of course right about xen_cpuid_base.

How about this:

>From 04b772d2b819f0dda2163e3193fa7cd447a6245c Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 27 Sep 2013 17:18:13 -0400
Subject: [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. 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'.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
[v1: ifdef xen_cpuid_base as it is X86 specific]
---
 drivers/tty/hvc/hvc_xen.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index e61c36c..6458c9f 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -183,7 +183,7 @@ static int dom0_write_console(uint32_t vtermno, const char *str, int len)
 {
 	int rc = HYPERVISOR_console_io(CONSOLEIO_write, len, (char *)str);
 	if (rc < 0)
-		return 0;
+		return rc;
 
 	return len;
 }
@@ -641,7 +641,22 @@ 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);
+#ifdef CONFIG_X86
+		if (rc == -ENOSYS && xen_hvm_domain())
+			goto outb_print;
+
+	} else if (xen_cpuid_base()) {
+		int i;
+outb_print:
+		for (i = 0; i < len; i++)
+			outb(str[i], 0xe9);
+#endif
+	}
 }
 
 void xen_raw_printk(const char *fmt, ...)
-- 
1.8.3.1

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Julien Grall]

On 09/30/2013 03:45 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 27, 2013 at 10:49:37PM +0100, Julien Grall wrote:
>> On 09/27/2013 10:25 PM, Konrad Rzeszutek Wilk wrote:
>>
>>> @@ -641,7 +641,20 @@ 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()) {
>>> +		dom0_write_console(0, str, len);
>>> +		if (rc == -ENOSYS && xen_hvm_domain())
>>> +			goto outb_print;
>>> +
>>> +	} else if (xen_cpuid_base()) {
>>> +		int i;
>>> +outb_print:
>>> +		for (i = 0; i < len; i++)
>>> +			outb(str[i], 0xe9);
>>> +	}
>>>   }
>>
>> xen_cpuid_base and outb(0xe9) is x86 specific and won't compile on ARM.
>
> Odd, I see outb defined in arch/arm and arch/arm64 ?(arch/arm[|64]/include/asm.io.h)

On ARM32 the IO access is memory mapped (the exact address depends on 
Linux configuration).
The main problem is not the outb macro but the ioport 0xe9.On ARM, this 
ioport is not trapped by Xen.

> You are of course right about xen_cpuid_base.
>
> How about this:

For the ARM side, the code looks good.

>  From 04b772d2b819f0dda2163e3193fa7cd447a6245c Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Fri, 27 Sep 2013 17:18:13 -0400
> Subject: [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. 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'.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> [v1: ifdef xen_cpuid_base as it is X86 specific]
> ---
>   drivers/tty/hvc/hvc_xen.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index e61c36c..6458c9f 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -183,7 +183,7 @@ static int dom0_write_console(uint32_t vtermno, const char *str, int len)
>   {
>   	int rc = HYPERVISOR_console_io(CONSOLEIO_write, len, (char *)str);
>   	if (rc < 0)
> -		return 0;
> +		return rc;
>
>   	return len;
>   }
> @@ -641,7 +641,22 @@ 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);
> +#ifdef CONFIG_X86
> +		if (rc == -ENOSYS && xen_hvm_domain())
> +			goto outb_print;
> +
> +	} else if (xen_cpuid_base()) {
> +		int i;
> +outb_print:
> +		for (i = 0; i < len; i++)
> +			outb(str[i], 0xe9);
> +#endif
> +	}
>   }
>
>   void xen_raw_printk(const char *fmt, ...)
>


-- 
Julien Grall

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Konrad Rzeszutek Wilk]

On Sun, Oct 06, 2013 at 09:52:40PM +0100, Julien Grall wrote:
> On 09/30/2013 03:45 PM, Konrad Rzeszutek Wilk wrote:
> >On Fri, Sep 27, 2013 at 10:49:37PM +0100, Julien Grall wrote:
> >>On 09/27/2013 10:25 PM, Konrad Rzeszutek Wilk wrote:
> >>
> >>>@@ -641,7 +641,20 @@ 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()) {
> >>>+		dom0_write_console(0, str, len);
> >>>+		if (rc == -ENOSYS && xen_hvm_domain())
> >>>+			goto outb_print;
> >>>+
> >>>+	} else if (xen_cpuid_base()) {
> >>>+		int i;
> >>>+outb_print:
> >>>+		for (i = 0; i < len; i++)
> >>>+			outb(str[i], 0xe9);
> >>>+	}
> >>>  }
> >>
> >>xen_cpuid_base and outb(0xe9) is x86 specific and won't compile on ARM.
> >
> >Odd, I see outb defined in arch/arm and arch/arm64 ?(arch/arm[|64]/include/asm.io.h)
> 
> On ARM32 the IO access is memory mapped (the exact address depends
> on Linux configuration).
> The main problem is not the outb macro but the ioport 0xe9.On ARM,
> this ioport is not trapped by Xen.
> 
> >You are of course right about xen_cpuid_base.
> >
> >How about this:
> 
> For the ARM side, the code looks good.

Can I that as 'Acked-by' ? thanks
> 
> > From 04b772d2b819f0dda2163e3193fa7cd447a6245c Mon Sep 17 00:00:00 2001
> >From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >Date: Fri, 27 Sep 2013 17:18:13 -0400
> >Subject: [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. 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'.
> >
> >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >[v1: ifdef xen_cpuid_base as it is X86 specific]
> >---
> >  drivers/tty/hvc/hvc_xen.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> >index e61c36c..6458c9f 100644
> >--- a/drivers/tty/hvc/hvc_xen.c
> >+++ b/drivers/tty/hvc/hvc_xen.c
> >@@ -183,7 +183,7 @@ static int dom0_write_console(uint32_t vtermno, const char *str, int len)
> >  {
> >  	int rc = HYPERVISOR_console_io(CONSOLEIO_write, len, (char *)str);
> >  	if (rc < 0)
> >-		return 0;
> >+		return rc;
> >
> >  	return len;
> >  }
> >@@ -641,7 +641,22 @@ 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);
> >+#ifdef CONFIG_X86
> >+		if (rc == -ENOSYS && xen_hvm_domain())
> >+			goto outb_print;
> >+
> >+	} else if (xen_cpuid_base()) {
> >+		int i;
> >+outb_print:
> >+		for (i = 0; i < len; i++)
> >+			outb(str[i], 0xe9);
> >+#endif
> >+	}
> >  }
> >
> >  void xen_raw_printk(const char *fmt, ...)
> >
> 
> 
> -- 
> Julien Grall

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Julien Grall]

On 10/23/2013 05:15 PM, Konrad Rzeszutek Wilk wrote:
> On Sun, Oct 06, 2013 at 09:52:40PM +0100, Julien Grall wrote:
>> On 09/30/2013 03:45 PM, Konrad Rzeszutek Wilk wrote:
>>> On Fri, Sep 27, 2013 at 10:49:37PM +0100, Julien Grall wrote:
>>>> On 09/27/2013 10:25 PM, Konrad Rzeszutek Wilk wrote:
>>>>
>>>>> @@ -641,7 +641,20 @@ 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()) {
>>>>> +		dom0_write_console(0, str, len);
>>>>> +		if (rc == -ENOSYS && xen_hvm_domain())
>>>>> +			goto outb_print;
>>>>> +
>>>>> +	} else if (xen_cpuid_base()) {
>>>>> +		int i;
>>>>> +outb_print:
>>>>> +		for (i = 0; i < len; i++)
>>>>> +			outb(str[i], 0xe9);
>>>>> +	}
>>>>>   }
>>>>
>>>> xen_cpuid_base and outb(0xe9) is x86 specific and won't compile on ARM.
>>>
>>> Odd, I see outb defined in arch/arm and arch/arm64 ?(arch/arm[|64]/include/asm.io.h)
>>
>> On ARM32 the IO access is memory mapped (the exact address depends
>> on Linux configuration).
>> The main problem is not the outb macro but the ioport 0xe9.On ARM,
>> this ioport is not trapped by Xen.
>>
>>> You are of course right about xen_cpuid_base.
>>>
>>> How about this:
>>
>> For the ARM side, the code looks good.
>
> Can I that as 'Acked-by' ? thanks

Actually, I looked closer the code, with the new solution 
xen_raw_printk/xen_raw_console_write can't be call on ARM during early init.

On ARM, xen_domain_type is initialized during a core initcall. So it's 
not possible to call the function before.

>>>  From 04b772d2b819f0dda2163e3193fa7cd447a6245c Mon Sep 17 00:00:00 2001
>>> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Date: Fri, 27 Sep 2013 17:18:13 -0400
>>> Subject: [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. 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'.
>>>
>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> [v1: ifdef xen_cpuid_base as it is X86 specific]
>>> ---
>>>   drivers/tty/hvc/hvc_xen.c | 19 +++++++++++++++++--
>>>   1 file changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
>>> index e61c36c..6458c9f 100644
>>> --- a/drivers/tty/hvc/hvc_xen.c
>>> +++ b/drivers/tty/hvc/hvc_xen.c
>>> @@ -183,7 +183,7 @@ static int dom0_write_console(uint32_t vtermno, const char *str, int len)
>>>   {
>>>   	int rc = HYPERVISOR_console_io(CONSOLEIO_write, len, (char *)str);
>>>   	if (rc < 0)
>>> -		return 0;
>>> +		return rc;
>>>
>>>   	return len;
>>>   }
>>> @@ -641,7 +641,22 @@ 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);
>>> +#ifdef CONFIG_X86
>>> +		if (rc == -ENOSYS && xen_hvm_domain())
>>> +			goto outb_print;
>>> +
>>> +	} else if (xen_cpuid_base()) {
>>> +		int i;
>>> +outb_print:
>>> +		for (i = 0; i < len; i++)
>>> +			outb(str[i], 0xe9);
>>> +#endif
>>> +	}
>>>   }
>>>
>>>   void xen_raw_printk(const char *fmt, ...)

-- 
Julien Grall

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Konrad Rzeszutek Wilk]

On Wed, Oct 23, 2013 at 11:08:01PM +0100, Julien Grall wrote:
> 
> 
> On 10/23/2013 05:15 PM, Konrad Rzeszutek Wilk wrote:
> >On Sun, Oct 06, 2013 at 09:52:40PM +0100, Julien Grall wrote:
> >>On 09/30/2013 03:45 PM, Konrad Rzeszutek Wilk wrote:
> >>>On Fri, Sep 27, 2013 at 10:49:37PM +0100, Julien Grall wrote:
> >>>>On 09/27/2013 10:25 PM, Konrad Rzeszutek Wilk wrote:
> >>>>
> >>>>>@@ -641,7 +641,20 @@ 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()) {
> >>>>>+		dom0_write_console(0, str, len);
> >>>>>+		if (rc == -ENOSYS && xen_hvm_domain())
> >>>>>+			goto outb_print;
> >>>>>+
> >>>>>+	} else if (xen_cpuid_base()) {
> >>>>>+		int i;
> >>>>>+outb_print:
> >>>>>+		for (i = 0; i < len; i++)
> >>>>>+			outb(str[i], 0xe9);
> >>>>>+	}
> >>>>>  }
> >>>>
> >>>>xen_cpuid_base and outb(0xe9) is x86 specific and won't compile on ARM.
> >>>
> >>>Odd, I see outb defined in arch/arm and arch/arm64 ?(arch/arm[|64]/include/asm.io.h)
> >>
> >>On ARM32 the IO access is memory mapped (the exact address depends
> >>on Linux configuration).
> >>The main problem is not the outb macro but the ioport 0xe9.On ARM,
> >>this ioport is not trapped by Xen.
> >>
> >>>You are of course right about xen_cpuid_base.
> >>>
> >>>How about this:
> >>
> >>For the ARM side, the code looks good.
> >
> >Can I that as 'Acked-by' ? thanks
> 
> Actually, I looked closer the code, with the new solution
> xen_raw_printk/xen_raw_console_write can't be call on ARM during
> early init.
> 
> On ARM, xen_domain_type is initialized during a core initcall. So
> it's not possible to call the function before.

OK, so won't work and won't harm. That is OK I think.
> 
> >>> From 04b772d2b819f0dda2163e3193fa7cd447a6245c Mon Sep 17 00:00:00 2001
> >>>From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >>>Date: Fri, 27 Sep 2013 17:18:13 -0400
> >>>Subject: [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. 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'.
> >>>
> >>>Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >>>[v1: ifdef xen_cpuid_base as it is X86 specific]
> >>>---
> >>>  drivers/tty/hvc/hvc_xen.c | 19 +++++++++++++++++--
> >>>  1 file changed, 17 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> >>>index e61c36c..6458c9f 100644
> >>>--- a/drivers/tty/hvc/hvc_xen.c
> >>>+++ b/drivers/tty/hvc/hvc_xen.c
> >>>@@ -183,7 +183,7 @@ static int dom0_write_console(uint32_t vtermno, const char *str, int len)
> >>>  {
> >>>  	int rc = HYPERVISOR_console_io(CONSOLEIO_write, len, (char *)str);
> >>>  	if (rc < 0)
> >>>-		return 0;
> >>>+		return rc;
> >>>
> >>>  	return len;
> >>>  }
> >>>@@ -641,7 +641,22 @@ 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);
> >>>+#ifdef CONFIG_X86
> >>>+		if (rc == -ENOSYS && xen_hvm_domain())
> >>>+			goto outb_print;
> >>>+
> >>>+	} else if (xen_cpuid_base()) {
> >>>+		int i;
> >>>+outb_print:
> >>>+		for (i = 0; i < len; i++)
> >>>+			outb(str[i], 0xe9);
> >>>+#endif
> >>>+	}
> >>>  }
> >>>
> >>>  void xen_raw_printk(const char *fmt, ...)
> 
> -- 
> Julien Grall

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Julien Grall]

On 10/24/2013 03:49 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 23, 2013 at 11:08:01PM +0100, Julien Grall wrote:
>>
>>
>> On 10/23/2013 05:15 PM, Konrad Rzeszutek Wilk wrote:
>>> On Sun, Oct 06, 2013 at 09:52:40PM +0100, Julien Grall wrote:
>> Actually, I looked closer the code, with the new solution
>> xen_raw_printk/xen_raw_console_write can't be call on ARM during
>> early init.
>>
>> On ARM, xen_domain_type is initialized during a core initcall. So
>> it's not possible to call the function before.
>
> OK, so won't work and won't harm. That is OK I think.

I use xen_raw_printk every time to debug early code and when someone has 
an issue with Linux (for instance no log in the console), we advise them 
to use xen_raw_printk.
It's annoying for every ARM developer to modify ourself the function 
each time we need it...

I'm sure the solution "move xen call earlier" will come up in few mails, 
so there was a discussion for swiotlb few weeks ago 
(https://lkml.org/lkml/2013/8/29/609). And the final decision was to 
avoid specific Xen call in arch_setup.

So I think, for now the best solution is to call unconditionally 
dom0_write_console on ARM (32 and 64 bits).

#ifdef CONFIG_X86
if (xen_domain) {
   ...
} else if (...) {
...
}
#else
    dom0_write_console
#endif

It will avoid to carry non-upstream patch for ARM and point out every 
time developer to this patch.

Stefano, Ian, any opinion?

-- 
Julien Grall

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Ian Campbell]

On Thu, 2013-10-24 at 17:30 +0100, Julien Grall wrote:

> I'm sure the solution "move xen call earlier" will come up in few mails, 
> so there was a discussion for swiotlb few weeks ago 
> (https://lkml.org/lkml/2013/8/29/609). And the final decision was to 
> avoid specific Xen call in arch_setup.

It wasn't so much a hard reject as a preference based on the need
presented at the time.

Perhaps a generic DT driven mechanism to detect hypervisors, called
early, would be more acceptable?

> Stefano, Ian, any opinion?

The ifdef approach is a bit ugly, but if the other option doesn't work
out I suppose we can live with it.

But stepping back how/why is this function called on x86 when not
running under Xen? Wouldn't the use have to be using console=hvc or
earlyprintk=xen or something -- which strikes me as user error... IOW
can we not just nuke the check

Ian.

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Konrad Rzeszutek Wilk]

On Thu, Oct 24, 2013 at 05:43:54PM +0100, Ian Campbell wrote:
> On Thu, 2013-10-24 at 17:30 +0100, Julien Grall wrote:
> 
> > I'm sure the solution "move xen call earlier" will come up in few mails, 
> > so there was a discussion for swiotlb few weeks ago 
> > (https://lkml.org/lkml/2013/8/29/609). And the final decision was to 
> > avoid specific Xen call in arch_setup.
> 
> It wasn't so much a hard reject as a preference based on the need
> presented at the time.
> 
> Perhaps a generic DT driven mechanism to detect hypervisors, called
> early, would be more acceptable?
> 
> > Stefano, Ian, any opinion?
> 
> The ifdef approach is a bit ugly, but if the other option doesn't work
> out I suppose we can live with it.
> 
> But stepping back how/why is this function called on x86 when not
> running under Xen? Wouldn't the use have to be using console=hvc or
> earlyprintk=xen or something -- which strikes me as user error... IOW
> can we not just nuke the check

It shouldn't - ever. On x86 it is called from the early PV bootup code.

I occasionaly use it when developing/debugging and as such want it working
in HVM even during early bootup.

> 
> Ian.
> 

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Ian Campbell]

On Fri, 2013-10-25 at 15:38 -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Oct 24, 2013 at 05:43:54PM +0100, Ian Campbell wrote:
> > But stepping back how/why is this function called on x86 when not
> > running under Xen? Wouldn't the use have to be using console=hvc or
> > earlyprintk=xen or something -- which strikes me as user error... IOW
> > can we not just nuke the check
> 
> It shouldn't - ever. On x86 it is called from the early PV bootup code.
> 
> I occasionaly use it when developing/debugging and as such want it working
> in HVM even during early bootup.

Does earlyprintk=ttyS0,cough not Just Work for HVM guests? That would
avoid all of this stuff and assuming it works would be preferable IMHO.

Anyway, if not, then given that the xen_cpuid_base case is entirely x86
specific perhaps we can abstract this as something like:

	if (xen_arch_raw_console(str))
		return

	dom0_write_console(...)

Then ARM:
	#define xen_arch_raw_console 0
and x86
	static inline int xen_arch_raw_console(str)
	{
		if (xen_domain())
			return 0;

		/* Do the outb thing */
		return 1;
	}

This would also avoid the need for #ifdef x86 in the main function which
Julien pointed out.

Ian.

Re: [Xen-devel] [PATCH] xen/hvc-console: Make it work with HVM guests.

[Konrad Rzeszutek Wilk]

On Sat, Oct 26, 2013 at 09:27:08AM +0100, Ian Campbell wrote:
> On Fri, 2013-10-25 at 15:38 -0400, Konrad Rzeszutek Wilk wrote:
> > On Thu, Oct 24, 2013 at 05:43:54PM +0100, Ian Campbell wrote:
> > > But stepping back how/why is this function called on x86 when not
> > > running under Xen? Wouldn't the use have to be using console=hvc or
> > > earlyprintk=xen or something -- which strikes me as user error... IOW
> > > can we not just nuke the check
> > 
> > It shouldn't - ever. On x86 it is called from the early PV bootup code.
> > 
> > I occasionaly use it when developing/debugging and as such want it working
> > in HVM even during early bootup.
> 
> Does earlyprintk=ttyS0,cough not Just Work for HVM guests? That would
> avoid all of this stuff and assuming it works would be preferable IMHO.

It works OK. But this is an alternative avenue of using 'xen_raw_printk'
for both PV and HVM.

> 
> Anyway, if not, then given that the xen_cpuid_base case is entirely x86
> specific perhaps we can abstract this as something like:
> 
> 	if (xen_arch_raw_console(str))
> 		return
> 
> 	dom0_write_console(...)
> 
> Then ARM:
> 	#define xen_arch_raw_console 0
> and x86
> 	static inline int xen_arch_raw_console(str)
> 	{
> 		if (xen_domain())
> 			return 0;
> 
> 		/* Do the outb thing */
> 		return 1;
> 	}
> 
> This would also avoid the need for #ifdef x86 in the main function which
> Julien pointed out.

OK.
> 
> Ian.
>