diff mbox series

[Xen-devel,14/17,v5] xen/arm: vpl011: Add support for vuart console in xenconsole

Message ID 1498117132-27139-15-git-send-email-bhupinder.thakur@linaro.org
State Superseded
Headers show
Series SBSA UART emulation support in Xen | expand

Commit Message

Bhupinder Thakur June 22, 2017, 7:38 a.m. UTC 
This patch finally adds the support for vuart console.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>

Changes since v4:
- Renamed VUART_CFLAGS- to CFLAGS_vuart- in the Makefile as per the convention.

 config/arm32.mk           |  1 +
 config/arm64.mk           |  1 +
 tools/console/Makefile    |  3 ++-
 tools/console/daemon/io.c | 31 ++++++++++++++++++++++++++++++-
 4 files changed, 34 insertions(+), 2 deletions(-)

Comments

Stefano Stabellini June 23, 2017, 12:02 a.m. UTC | #1 
On Thu, 22 Jun 2017, Bhupinder Thakur wrote:
> This patch finally adds the support for vuart console.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> 
> Changes since v4:
> - Renamed VUART_CFLAGS- to CFLAGS_vuart- in the Makefile as per the convention.
> 
>  config/arm32.mk           |  1 +
>  config/arm64.mk           |  1 +
>  tools/console/Makefile    |  3 ++-
>  tools/console/daemon/io.c | 31 ++++++++++++++++++++++++++++++-
>  4 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/config/arm32.mk b/config/arm32.mk
> index f95228e..b9f23fe 100644
> --- a/config/arm32.mk
> +++ b/config/arm32.mk
> @@ -1,5 +1,6 @@
>  CONFIG_ARM := y
>  CONFIG_ARM_32 := y
> +CONFIG_VUART_CONSOLE := y
>  CONFIG_ARM_$(XEN_OS) := y

I am tempted to disable this by default on arm32 (but leaving it
configurable via Kconfig maybe). Tipically arm32 cpus are not found on
server platforms, where SBSA compliance is important. Julien, what do
you think?


>  CONFIG_XEN_INSTALL_SUFFIX :=
> diff --git a/config/arm64.mk b/config/arm64.mk
> index aa45772..861d0a4 100644
> --- a/config/arm64.mk
> +++ b/config/arm64.mk
> @@ -1,5 +1,6 @@
>  CONFIG_ARM := y
>  CONFIG_ARM_64 := y
> +CONFIG_VUART_CONSOLE := y
>  CONFIG_ARM_$(XEN_OS) := y
>  
>  CONFIG_XEN_INSTALL_SUFFIX :=
> diff --git a/tools/console/Makefile b/tools/console/Makefile
> index c8b0300..1cddb6e 100644
> --- a/tools/console/Makefile
> +++ b/tools/console/Makefile
> @@ -11,6 +11,7 @@ LDLIBS += $(SOCKET_LIBS)
>  
>  LDLIBS_xenconsoled += $(UTIL_LIBS)
>  LDLIBS_xenconsoled += -lrt
> +CFLAGS_vuart-$(CONFIG_VUART_CONSOLE) = -DCONFIG_VUART_CONSOLE
>  
>  BIN      = xenconsoled xenconsole
>  
> @@ -28,7 +29,7 @@ clean:
>  distclean: clean
>  
>  daemon/main.o: daemon/_paths.h
> -daemon/io.o: CFLAGS += $(CFLAGS_libxenevtchn) $(CFLAGS_libxengnttab)
> +daemon/io.o: CFLAGS += $(CFLAGS_libxenevtchn) $(CFLAGS_libxengnttab) $(CFLAGS_vuart-y)
>  xenconsoled: $(patsubst %.c,%.o,$(wildcard daemon/*.c))
>  	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS) $(LDLIBS_libxenevtchn) $(LDLIBS_libxengnttab) $(LDLIBS_xenconsoled) $(APPEND_LDFLAGS)
>  
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index baf0e2e..6b0114e 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -107,12 +107,16 @@ struct console {
>  	xenevtchn_port_or_error_t remote_port;
>  	struct xencons_interface *interface;
>  	struct domain *d;
> +	bool optional;
> +	bool prefer_gnttab;
>  };
>  
>  struct console_data {
>  	char *xsname;
>  	char *ttyname;
>  	char *log_suffix;
> +	bool optional;
> +	bool prefer_gnttab;
>  };
>  
>  static struct console_data console_data[] = {
> @@ -121,7 +125,18 @@ static struct console_data console_data[] = {
>  		.xsname = "/console",
>  		.ttyname = "tty",
>  		.log_suffix = "",
> +		.optional = false,
> +		.prefer_gnttab = true,
>  	},
> +#if defined(CONFIG_VUART_CONSOLE)
> +	{
> +		.xsname = "/vuart/0",
> +		.ttyname = "tty",
> +		.log_suffix = "-vuart0",
> +		.optional = true,
> +		.prefer_gnttab = false,
> +	},
> +#endif
>  };
>  
>  #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
> @@ -655,8 +670,18 @@ static int console_create_ring(struct console *con)
>  			"ring-ref", "%u", &ring_ref,
>  			"port", "%i", &remote_port,
>  			NULL);
> +
>  	if (err)
> +	{
> +		/*
> +		 * This is a normal condition for optional consoles: they might not be
> +		 * present on xenstore at all. In that case, just return without error.
> +		*/
> +		if (con->optional)
> +			err = 0;
> +
>  		goto out;
> +	}
>  
>  	snprintf(path, sizeof(path), "%s/type", con->xspath);
>  	type = xs_read(xs, XBT_NULL, path, NULL);
> @@ -670,7 +695,9 @@ static int console_create_ring(struct console *con)
>  	if (ring_ref != con->ring_ref && con->ring_ref != -1)
>  		console_unmap_interface(con);
>  
> -	if (!con->interface && xgt_handle) {
> +	if (!con->interface && 
> +		xgt_handle &&
> +		con->prefer_gnttab) {
>  		/* Prefer using grant table */
>  		con->interface = xengnttab_map_grant_ref(xgt_handle,
>  			dom->domid, GNTTAB_RESERVED_CONSOLE,
> @@ -790,6 +817,8 @@ static int console_init(struct console *con, struct domain *dom, void **data)
>  	con->d = dom;
>  	con->ttyname = (*con_data)->ttyname;
>  	con->log_suffix = (*con_data)->log_suffix;
> +	con->optional = (*con_data)->optional;
> +	con->prefer_gnttab = (*con_data)->prefer_gnttab;
>  	xsname = (*con_data)->xsname;
>  	con->xspath = xs_get_domain_path(xs, dom->domid);
>  	s = realloc(con->xspath, strlen(con->xspath) +
> -- 
> 2.7.4
>
Wei Liu June 28, 2017, 5:17 p.m. UTC | #2 
On Thu, Jun 22, 2017 at 01:08:49PM +0530, Bhupinder Thakur wrote:
> This patch finally adds the support for vuart console.
> 
[...]
>  
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index baf0e2e..6b0114e 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -107,12 +107,16 @@ struct console {
>  	xenevtchn_port_or_error_t remote_port;
>  	struct xencons_interface *interface;
>  	struct domain *d;
> +	bool optional;
> +	bool prefer_gnttab;

Please state in commit message or code comment what these two fields do
and how they are supposed to be set?
diff mbox series

Patch

diff --git a/config/arm32.mk b/config/arm32.mk
index f95228e..b9f23fe 100644
--- a/config/arm32.mk
+++ b/config/arm32.mk
@@ -1,5 +1,6 @@ 
 CONFIG_ARM := y
 CONFIG_ARM_32 := y
+CONFIG_VUART_CONSOLE := y
 CONFIG_ARM_$(XEN_OS) := y
 
 CONFIG_XEN_INSTALL_SUFFIX :=
diff --git a/config/arm64.mk b/config/arm64.mk
index aa45772..861d0a4 100644
--- a/config/arm64.mk
+++ b/config/arm64.mk
@@ -1,5 +1,6 @@ 
 CONFIG_ARM := y
 CONFIG_ARM_64 := y
+CONFIG_VUART_CONSOLE := y
 CONFIG_ARM_$(XEN_OS) := y
 
 CONFIG_XEN_INSTALL_SUFFIX :=
diff --git a/tools/console/Makefile b/tools/console/Makefile
index c8b0300..1cddb6e 100644
--- a/tools/console/Makefile
+++ b/tools/console/Makefile
@@ -11,6 +11,7 @@  LDLIBS += $(SOCKET_LIBS)
 
 LDLIBS_xenconsoled += $(UTIL_LIBS)
 LDLIBS_xenconsoled += -lrt
+CFLAGS_vuart-$(CONFIG_VUART_CONSOLE) = -DCONFIG_VUART_CONSOLE
 
 BIN      = xenconsoled xenconsole
 
@@ -28,7 +29,7 @@  clean:
 distclean: clean
 
 daemon/main.o: daemon/_paths.h
-daemon/io.o: CFLAGS += $(CFLAGS_libxenevtchn) $(CFLAGS_libxengnttab)
+daemon/io.o: CFLAGS += $(CFLAGS_libxenevtchn) $(CFLAGS_libxengnttab) $(CFLAGS_vuart-y)
 xenconsoled: $(patsubst %.c,%.o,$(wildcard daemon/*.c))
 	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS) $(LDLIBS_libxenevtchn) $(LDLIBS_libxengnttab) $(LDLIBS_xenconsoled) $(APPEND_LDFLAGS)
 
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index baf0e2e..6b0114e 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -107,12 +107,16 @@  struct console {
 	xenevtchn_port_or_error_t remote_port;
 	struct xencons_interface *interface;
 	struct domain *d;
+	bool optional;
+	bool prefer_gnttab;
 };
 
 struct console_data {
 	char *xsname;
 	char *ttyname;
 	char *log_suffix;
+	bool optional;
+	bool prefer_gnttab;
 };
 
 static struct console_data console_data[] = {
@@ -121,7 +125,18 @@  static struct console_data console_data[] = {
 		.xsname = "/console",
 		.ttyname = "tty",
 		.log_suffix = "",
+		.optional = false,
+		.prefer_gnttab = true,
 	},
+#if defined(CONFIG_VUART_CONSOLE)
+	{
+		.xsname = "/vuart/0",
+		.ttyname = "tty",
+		.log_suffix = "-vuart0",
+		.optional = true,
+		.prefer_gnttab = false,
+	},
+#endif
 };
 
 #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
@@ -655,8 +670,18 @@  static int console_create_ring(struct console *con)
 			"ring-ref", "%u", &ring_ref,
 			"port", "%i", &remote_port,
 			NULL);
+
 	if (err)
+	{
+		/*
+		 * This is a normal condition for optional consoles: they might not be
+		 * present on xenstore at all. In that case, just return without error.
+		*/
+		if (con->optional)
+			err = 0;
+
 		goto out;
+	}
 
 	snprintf(path, sizeof(path), "%s/type", con->xspath);
 	type = xs_read(xs, XBT_NULL, path, NULL);
@@ -670,7 +695,9 @@  static int console_create_ring(struct console *con)
 	if (ring_ref != con->ring_ref && con->ring_ref != -1)
 		console_unmap_interface(con);
 
-	if (!con->interface && xgt_handle) {
+	if (!con->interface && 
+		xgt_handle &&
+		con->prefer_gnttab) {
 		/* Prefer using grant table */
 		con->interface = xengnttab_map_grant_ref(xgt_handle,
 			dom->domid, GNTTAB_RESERVED_CONSOLE,
@@ -790,6 +817,8 @@  static int console_init(struct console *con, struct domain *dom, void **data)
 	con->d = dom;
 	con->ttyname = (*con_data)->ttyname;
 	con->log_suffix = (*con_data)->log_suffix;
+	con->optional = (*con_data)->optional;
+	con->prefer_gnttab = (*con_data)->prefer_gnttab;
 	xsname = (*con_data)->xsname;
 	con->xspath = xs_get_domain_path(xs, dom->domid);
 	s = realloc(con->xspath, strlen(con->xspath) +