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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 1 May 2017, Bhupinder Thakur wrote:
> Hi Stefano,
> 
> >> 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.
> >
> I have a doubt here. Do I have to create a new console device
> (libxl__device) to register the vuart console or can I use the
> existing console device (which is used for registering the primary
> console) to register the vuart nodes to xenstore?
> 
> I suspect that if I try to register with the same console device then
> libxl__device_generic_add() may fail as it is already added.

The vuart we are introducing with this patch series is a new and
different console from the existing PV console. So yes, I think we need
to create a new device for it.

We either need to introduce a brand new function to add the vuart to
xenstore, something like libxl__device_vuart_add, or, if we are going
to reuse libxl__device_console_add, then we we need to create both a new
console device (libxl__device_console), and a new libxl__device for it.
For clarity, libxl__device_console is the input parameter of
libxl__device_console_add, while libxl__device is the output parameter.


> >>      } 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
> Ok. I will move these changes to the patch#4.
> 
> Regards,
> Bhupinder
> 

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