Re: [PATCH 06/10 v2] xen/arm: vpl011: Add vuart ring-buf and evtchn to xenstore

From: Stefano Stabellini <sstabellini@kernel.org>
To: Bhupinder Thakur <bhupinder.thakur@linaro.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 06/10 v2] xen/arm: vpl011: Add vuart ring-buf and evtchn to xenstore
Date: Fri, 28 Apr 2017 14:57:25 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1704281452100.2895@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <1493395284-18430-7-git-send-email-bhupinder.thakur@linaro.org>

On Fri, 28 Apr 2017, Bhupinder Thakur wrote:
> Add two new parameters to the xen store to be used by xenconsoled:
>     - newly allocated PFN to be used as IN/OUT ring buffer
>     - get a new event channel allocated by Xen using a domctl call
> 
> These paramters are added to xenstore only if vuart console is enabled
> by the user.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> 
> Changes since v1:
> - Modified the xenstore key names to /vuart/0/ring-ref and 
>   /vuart/0/port.
> - Replaced the hvm call with domctl call to get the event channel.
> 
>  tools/libxl/libxl_console.c | 10 ++++++++++
>  tools/libxl/libxl_dom.c     |  4 ++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
> index 446e766..ef3bd44 100644
> --- a/tools/libxl/libxl_console.c
> +++ b/tools/libxl/libxl_console.c
> @@ -67,6 +67,9 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
>      case LIBXL_CONSOLE_TYPE_SERIAL:
>          cons_type_s = "serial";
>          break;
> +    case LIBXL_CONSOLE_TYPE_VUART:
> +        cons_type_s = "vuart";
> +        break;
>      default:
>          goto out;
>      }
> @@ -326,6 +329,13 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
>          flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->console_port));
>          flexarray_append(ro_front, "ring-ref");
>          flexarray_append(ro_front, GCSPRINTF("%lu", state->console_mfn));
> +        if (state->vuart_enabled)
> +        {
> +            flexarray_append(ro_front, "vuart/0/port");
> +            flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vuart_port));
> +            flexarray_append(ro_front, "vuart/0/ring-ref");
> +            flexarray_append(ro_front, GCSPRINTF("%lu", state->vuart_mfn));
> +        }

It looks like you are reusing the libxl__device_console_add call for the
main PV console for the domain, to also add the vuart nodes to xenstore.

I don't think it is a good idea to mix the two. I suggest to introduce a
new libxl__device call to introduce the vuart nodes to xenstore, given
that they have no relantionship with the principal PV console of the
domain.

>      } else {
>          flexarray_append(front, "state");
>          flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising));
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 5d914a5..06ff3b7 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -434,6 +434,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>      state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid);
>      state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid);
>  
> +    if (state->vuart_enabled)
> +        xc_domain_vuart_get_evtchn(ctx->xch, domid, &state->vuart_port);
> +
>      if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
>          hvm_set_conf_params(ctx->xch, domid, info);
>  #if defined(__i386__) || defined(__x86_64__)
> @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>      if (xc_dom_translated(dom)) {
>          state->console_mfn = dom->console_pfn;
>          state->store_mfn = dom->xenstore_pfn;
> +        state->vuart_mfn = dom->vuart_pfn;
>      } else {
>          state->console_mfn = xc_dom_p2m(dom, dom->console_pfn);
>          state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn);

These two changes to libxl_dom.c probably belong to patch #4

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