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

[Julien Grall <julien.grall@linaro.org>]

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