Re: [PATCH 11/14 v4] xen/arm: vpl011: Add support for vuart console in xenconsole

[Stefano Stabellini <sstabellini@kernel.org>]

On Tue, 6 Jun 2017, Bhupinder Thakur wrote:
> This patch finally adds the support for vuart console.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
>
> ---
> CC: ij
> CC: wl
> CC: ss
> CC: jg
> 
> Changes since v3:
> - The changes in xenconsole have been split into four patches. This is the fourth patch.
> - The vuart console support is added under CONFIG_VUART_CONSOLE option.
> 
>  tools/console/Makefile    |  3 ++-
>  tools/console/daemon/io.c | 26 +++++++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/console/Makefile b/tools/console/Makefile
> index c8b0300..fcee313 100644
> --- a/tools/console/Makefile
> +++ b/tools/console/Makefile
> @@ -11,6 +11,7 @@ LDLIBS += $(SOCKET_LIBS)
>  
>  LDLIBS_xenconsoled += $(UTIL_LIBS)
>  LDLIBS_xenconsoled += -lrt
> +VUART_CFLAGS-$(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) $(VUART_CFLAGS-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 db73e10..cae2efe 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -104,12 +104,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[] = {
> @@ -118,7 +122,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))
> @@ -665,7 +680,12 @@ static int console_create_ring(struct console *con)
>  			"port", "%i", &remote_port,
>  			NULL);
>  	if (err)
> +	{
> +		/* If the console is optional then do not return an error. */
> +		if (con->optional)
> +			err = 0;
>  		goto out;
> +	}

If the console is optional, this function gets called, but we might find
nothing on xenstore, because the console doesn't actually exist. Thus,
we don't want to return error, but just return. Is that right?

If so, please rephrase this comment:

/* 
 * This is a normal condition for optional consoles: they might not be
 * present on xenstore at all. In that case, just return without error.
 */

The rest looks fine.


>  	snprintf(path, sizeof(path), "%s/type", con->xspath);
>  	type = xs_read(xs, XBT_NULL, path, NULL);
> @@ -679,7 +699,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,
> @@ -789,6 +811,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;
>  	con->xsname = (*con_data)->xsname;
>  	con->xspath = xs_get_domain_path(xs, dom->domid);
>  	s = realloc(con->xspath, strlen(con->xspath) +
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel