diff mbox series

[Xen-devel,09/12,v3] xen/arm: vpl011: Add support for vuart in xenconsole

Message ID 1494426918-32737-4-git-send-email-bhupinder.thakur@linaro.org
State New
Headers show
Series None | expand

Commit Message

Bhupinder Thakur May 10, 2017, 2:35 p.m. UTC
Xenconsole supports only PV console currently. This patch adds support
for vuart console, which allows emulated pl011 UART to be accessed
as a console.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---

One review comment was to keep the vuart code under CONFIG_ARM64 && CONFIG_ACPI flags.
This code review could not be incorporated as I could not find out the appropriate flags
unders which this code can be kept. Are the CONFIG* flags exported to xenconsole?

 tools/console/daemon/io.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

Wei Liu May 11, 2017, 11:17 a.m. UTC | #1
On Wed, May 10, 2017 at 08:05:15PM +0530, Bhupinder Thakur wrote:
> Xenconsole supports only PV console currently. This patch adds support
> for vuart console, which allows emulated pl011 UART to be accessed
> as a console.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> 
> One review comment was to keep the vuart code under CONFIG_ARM64 && CONFIG_ACPI flags.
> This code review could not be incorporated as I could not find out the appropriate flags
> unders which this code can be kept. Are the CONFIG* flags exported to xenconsole?

IIRC they are exported.

> 
>  tools/console/daemon/io.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 9bb14de..19a2f35 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -115,6 +115,7 @@ struct console_data {
>  };
>  
>  static int map_pvcon_ring_ref(struct console *, int );
> +static int map_vuartcon_ring_ref(struct console *, int );
>  
>  static struct console_data console_data[] = {
>  
> @@ -124,6 +125,12 @@ static struct console_data console_data[] = {
>  		.mapfunc = map_pvcon_ring_ref,
>  		.mandatory = true
>  	},
> +	{
> +		.xsname = "/vuart/0",
> +		.ttyname = "tty",
> +		.mapfunc = map_vuartcon_ring_ref,
> +		.mandatory = false
> +	}
>  };
>  
>  #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
> @@ -751,6 +758,28 @@ out:
>  	return err;
>  }
>  
> +static int map_vuartcon_ring_ref(struct console *con, int ring_ref)
> +{
> +	int err = 0;
> +	struct domain *dom = con->d;
> +
> +	if (!con->interface) {
> +		con->interface = xc_map_foreign_range(xc,
> +											  dom->domid,
> +											  XC_PAGE_SIZE,
> +											  PROT_READ|PROT_WRITE,
> +											  (unsigned long)ring_ref);

Indentation.

> +		if (con->interface == NULL) {
> +			err = EINVAL;
> +			goto out;
> +		}
> +		con->ring_ref = ring_ref;
> +	}
> +
> +out:
> +	return err;
> +}
> +
>  static int console_create_ring(struct console *con)
>  {
>  	int err, remote_port, ring_ref;
> -- 
> 2.7.4
>
Stefano Stabellini May 16, 2017, 11:44 p.m. UTC | #2
On Wed, 10 May 2017, Bhupinder Thakur wrote:
> Xenconsole supports only PV console currently. This patch adds support
> for vuart console, which allows emulated pl011 UART to be accessed
> as a console.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> 
> One review comment was to keep the vuart code under CONFIG_ARM64 && CONFIG_ACPI flags.
> This code review could not be incorporated as I could not find out the appropriate flags
> unders which this code can be kept. Are the CONFIG* flags exported to xenconsole?

Not sure about ACPI, but CONFIG_ARM_64 definitely should be.

This patch looks good, however I don't feel confident giving my acked-by
until I can review properly the previous one.


>  tools/console/daemon/io.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 9bb14de..19a2f35 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -115,6 +115,7 @@ struct console_data {
>  };
>  
>  static int map_pvcon_ring_ref(struct console *, int );
> +static int map_vuartcon_ring_ref(struct console *, int );
>  
>  static struct console_data console_data[] = {
>  
> @@ -124,6 +125,12 @@ static struct console_data console_data[] = {
>  		.mapfunc = map_pvcon_ring_ref,
>  		.mandatory = true
>  	},
> +	{
> +		.xsname = "/vuart/0",
> +		.ttyname = "tty",
> +		.mapfunc = map_vuartcon_ring_ref,
> +		.mandatory = false
> +	}
>  };
>  
>  #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
> @@ -751,6 +758,28 @@ out:
>  	return err;
>  }
>  
> +static int map_vuartcon_ring_ref(struct console *con, int ring_ref)
> +{
> +	int err = 0;
> +	struct domain *dom = con->d;
> +
> +	if (!con->interface) {
> +		con->interface = xc_map_foreign_range(xc,
> +											  dom->domid,
> +											  XC_PAGE_SIZE,
> +											  PROT_READ|PROT_WRITE,
> +											  (unsigned long)ring_ref);
> +		if (con->interface == NULL) {
> +			err = EINVAL;
> +			goto out;
> +		}
> +		con->ring_ref = ring_ref;
> +	}
> +
> +out:
> +	return err;
> +}
> +
>  static int console_create_ring(struct console *con)
>  {
>  	int err, remote_port, ring_ref;
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
Julien Grall May 22, 2017, 2:37 p.m. UTC | #3
Hi,

On 17/05/17 00:44, Stefano Stabellini wrote:
> On Wed, 10 May 2017, Bhupinder Thakur wrote:
>> Xenconsole supports only PV console currently. This patch adds support
>> for vuart console, which allows emulated pl011 UART to be accessed
>> as a console.
>>
>> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
>> ---
>>
>> One review comment was to keep the vuart code under CONFIG_ARM64 && CONFIG_ACPI flags.
>> This code review could not be incorporated as I could not find out the appropriate flags
>> unders which this code can be kept. Are the CONFIG* flags exported to xenconsole?
>
> Not sure about ACPI, but CONFIG_ARM_64 definitely should be.

Why only CONFIG_ARM_64? I don't see any restriction preventing to be 
used for ARM32 bit also.

If we decide to limit to ARM_64 here, then we should do the same on the 
hypervisor side.

But I am wondering whether we should have a configure option for that as 
the hypervisor may not support pl011 (the user is allowed to disable it).

Cheers,

>
> This patch looks good, however I don't feel confident giving my acked-by
> until I can review properly the previous one.
>
>
>>  tools/console/daemon/io.c | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
>> index 9bb14de..19a2f35 100644
>> --- a/tools/console/daemon/io.c
>> +++ b/tools/console/daemon/io.c
>> @@ -115,6 +115,7 @@ struct console_data {
>>  };
>>
>>  static int map_pvcon_ring_ref(struct console *, int );
>> +static int map_vuartcon_ring_ref(struct console *, int );
>>
>>  static struct console_data console_data[] = {
>>
>> @@ -124,6 +125,12 @@ static struct console_data console_data[] = {
>>  		.mapfunc = map_pvcon_ring_ref,
>>  		.mandatory = true
>>  	},
>> +	{
>> +		.xsname = "/vuart/0",
>> +		.ttyname = "tty",
>> +		.mapfunc = map_vuartcon_ring_ref,
>> +		.mandatory = false
>> +	}
>>  };
>>
>>  #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
>> @@ -751,6 +758,28 @@ out:
>>  	return err;
>>  }
>>
>> +static int map_vuartcon_ring_ref(struct console *con, int ring_ref)
>> +{
>> +	int err = 0;
>> +	struct domain *dom = con->d;
>> +
>> +	if (!con->interface) {
>> +		con->interface = xc_map_foreign_range(xc,
>> +											  dom->domid,
>> +											  XC_PAGE_SIZE,
>> +											  PROT_READ|PROT_WRITE,
>> +											  (unsigned long)ring_ref);
>> +		if (con->interface == NULL) {
>> +			err = EINVAL;
>> +			goto out;
>> +		}
>> +		con->ring_ref = ring_ref;
>> +	}
>> +
>> +out:
>> +	return err;
>> +}
>> +
>>  static int console_create_ring(struct console *con)
>>  {
>>  	int err, remote_port, ring_ref;
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
>>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
diff mbox series

Patch

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 9bb14de..19a2f35 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -115,6 +115,7 @@  struct console_data {
 };
 
 static int map_pvcon_ring_ref(struct console *, int );
+static int map_vuartcon_ring_ref(struct console *, int );
 
 static struct console_data console_data[] = {
 
@@ -124,6 +125,12 @@  static struct console_data console_data[] = {
 		.mapfunc = map_pvcon_ring_ref,
 		.mandatory = true
 	},
+	{
+		.xsname = "/vuart/0",
+		.ttyname = "tty",
+		.mapfunc = map_vuartcon_ring_ref,
+		.mandatory = false
+	}
 };
 
 #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
@@ -751,6 +758,28 @@  out:
 	return err;
 }
 
+static int map_vuartcon_ring_ref(struct console *con, int ring_ref)
+{
+	int err = 0;
+	struct domain *dom = con->d;
+
+	if (!con->interface) {
+		con->interface = xc_map_foreign_range(xc,
+											  dom->domid,
+											  XC_PAGE_SIZE,
+											  PROT_READ|PROT_WRITE,
+											  (unsigned long)ring_ref);
+		if (con->interface == NULL) {
+			err = EINVAL;
+			goto out;
+		}
+		con->ring_ref = ring_ref;
+	}
+
+out:
+	return err;
+}
+
 static int console_create_ring(struct console *con)
 {
 	int err, remote_port, ring_ref;