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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
> 
> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel