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

[Bhupinder Thakur <bhupinder.thakur@linaro.org>]

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.

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