| Message ID | 1487676368-22356-8-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | pl011 emulation support in Xen | expand |
On Tue, Feb 21, 2017 at 04:56:04PM +0530, Bhupinder Thakur wrote: > Add two new parameters to the xen store: > - newly allocated PFN to be used as IN/OUT ring buffer by xenconsoled > - a new event channel read from Xen using a hvm call to be used by xenconsoled > for eventing This should have a correspoinding change in the include/public/io/console.h describing this new 'vpl011-port' and 'vpl011-ring-ref' Xenbus entry. Feel free to lift from kbdif.h. Unless an earlier patch already does this? In which case you should mention it: "Patch titled XYZ introduces these XenBus entries in console.h" > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > --- > tools/libxl/libxl.c | 6 ++++++ > tools/libxl/libxl_dom.c | 18 +++++++++++++++++- > tools/libxl/libxl_internal.h | 4 ++++ > 3 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index d400fa2..cc00235 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -3159,6 +3159,12 @@ 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 defined(__arm__) || defined(__aarch64__) > + flexarray_append(ro_front, "vpl011-port"); > + flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vpl011_console_port)); > + flexarray_append(ro_front, "vpl011-ring-ref"); > + flexarray_append(ro_front, GCSPRINTF("%lu", state->vpl011_console_mfn)); > +#endif so... what if the VPL011 is not enabled in the guest? Should we still create these XenBus entries? > } 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 d519c8d..39eaff6 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -302,7 +302,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > libxl_ctx *ctx = libxl__gc_owner(gc); > char *xs_domid, *con_domid; > int rc; > - uint64_t size; > + uint64_t size, val=-1; So -1 for uint64_t is 0xffffffff... Is that what you meant? And why? Why not not uint32_t? > > if (xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus) != 0) { > LOG(ERROR, "Couldn't set max vcpu count"); > @@ -432,6 +432,16 @@ 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 defined(__arm__) || defined(__aarch64__) > + /* get the vpl011 event channel from Xen */ Please remove this comment. > + rc = xc_hvm_param_get(ctx->xch, domid, HVM_PARAM_VPL011_CONSOLE_EVTCHN, > + &val); > + if ( rc ) > + state->vpl011_console_port = -1; > + else > + state->vpl011_console_port = (uint32_t)val; > +#endif > + > if (info->type == LIBXL_DOMAIN_TYPE_HVM) { > hvm_set_conf_params(ctx->xch, domid, info); > #if defined(__i386__) || defined(__x86_64__) > @@ -727,6 +737,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, > > dom->flags = flags; > dom->console_evtchn = state->console_port; > +#if defined(__arm__) || defined(__aarch64__) > + dom->vpl011_console_evtchn = state->vpl011_console_port; > +#endif > dom->console_domid = state->console_domid; > dom->xenstore_evtchn = state->store_port; > dom->xenstore_domid = state->store_domid; > @@ -771,6 +784,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, > if (xc_dom_feature_translated(dom)) { > state->console_mfn = dom->console_pfn; > state->store_mfn = dom->xenstore_pfn; > +#if defined(__arm__) || defined(__aarch64__) > + state->vpl011_console_mfn = dom->vpl011_console_pfn; > +#endif > } else { > state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); > state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 5f46578..10e262e 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -1128,6 +1128,10 @@ typedef struct { > uint32_t num_vmemranges; > > xc_domain_configuration_t config; > +#if defined(__arm__) || defined(__aarch64__) > + unsigned long vpl011_console_mfn; > +#endif I am not a big fan of these #ifdef. Could they go away and this could would be compiled on x86 too but just never used? (The default value to use this owuld be disabled for example)? > } libxl__domain_build_state; > > _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid, > -- > 2.7.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
Hi Konrad, >> Add two new parameters to the xen store: >> - newly allocated PFN to be used as IN/OUT ring buffer by xenconsoled >> - a new event channel read from Xen using a hvm call to be used by xenconsoled >> for eventing > > This should have a correspoinding change in the include/public/io/console.h > describing this new 'vpl011-port' and 'vpl011-ring-ref' Xenbus entry. > > Feel free to lift from kbdif.h. > I tried looking at kbdif.h but could not find any xenbus entry definitions. I looked at how "ring-ref" is defined which is used for the PV console. I tried to define "vpl011-ring-ref" in a similar way. > Unless an earlier patch already does this? In which case you should > mention it: > > "Patch titled XYZ introduces these XenBus entries in console.h" > >> >> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> >> --- >> tools/libxl/libxl.c | 6 ++++++ >> tools/libxl/libxl_dom.c | 18 +++++++++++++++++- >> tools/libxl/libxl_internal.h | 4 ++++ >> 3 files changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >> index d400fa2..cc00235 100644 >> --- a/tools/libxl/libxl.c >> +++ b/tools/libxl/libxl.c >> @@ -3159,6 +3159,12 @@ 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 defined(__arm__) || defined(__aarch64__) >> + flexarray_append(ro_front, "vpl011-port"); >> + flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vpl011_console_port)); >> + flexarray_append(ro_front, "vpl011-ring-ref"); >> + flexarray_append(ro_front, GCSPRINTF("%lu", state->vpl011_console_mfn)); >> +#endif > > so... what if the VPL011 is not enabled in the guest? Should we still > create these XenBus entries? I will add a libxl option for enabling/disabling pl011 emulation. Only when this option is enabled, vpl011-port and vpl011-ring-ref would be added to xenstore. > >> } 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 d519c8d..39eaff6 100644 >> --- a/tools/libxl/libxl_dom.c >> +++ b/tools/libxl/libxl_dom.c >> @@ -302,7 +302,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, >> libxl_ctx *ctx = libxl__gc_owner(gc); >> char *xs_domid, *con_domid; >> int rc; >> - uint64_t size; >> + uint64_t size, val=-1; > > So -1 for uint64_t is 0xffffffff... > > Is that what you meant? And why? Why not not uint32_t? I used -1 to indicate invalid event_channel since 0 might be a valid event channel number. Since xc_hvm_param_get() expects a uint64_t, I used a local variable of type unit64_t. > > >> >> if (xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus) != 0) { >> LOG(ERROR, "Couldn't set max vcpu count"); >> @@ -432,6 +432,16 @@ 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 defined(__arm__) || defined(__aarch64__) >> + /* get the vpl011 event channel from Xen */ > > Please remove this comment. Ok. > >> + rc = xc_hvm_param_get(ctx->xch, domid, HVM_PARAM_VPL011_CONSOLE_EVTCHN, >> + &val); >> + if ( rc ) >> + state->vpl011_console_port = -1; >> + else >> + state->vpl011_console_port = (uint32_t)val; >> +#endif >> + >> if (info->type == LIBXL_DOMAIN_TYPE_HVM) { >> hvm_set_conf_params(ctx->xch, domid, info); >> #if defined(__i386__) || defined(__x86_64__) >> @@ -727,6 +737,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, >> >> dom->flags = flags; >> dom->console_evtchn = state->console_port; >> +#if defined(__arm__) || defined(__aarch64__) >> + dom->vpl011_console_evtchn = state->vpl011_console_port; >> +#endif >> dom->console_domid = state->console_domid; >> dom->xenstore_evtchn = state->store_port; >> dom->xenstore_domid = state->store_domid; >> @@ -771,6 +784,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, >> if (xc_dom_feature_translated(dom)) { >> state->console_mfn = dom->console_pfn; >> state->store_mfn = dom->xenstore_pfn; >> +#if defined(__arm__) || defined(__aarch64__) >> + state->vpl011_console_mfn = dom->vpl011_console_pfn; >> +#endif >> } else { >> state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); >> state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h >> index 5f46578..10e262e 100644 >> --- a/tools/libxl/libxl_internal.h >> +++ b/tools/libxl/libxl_internal.h >> @@ -1128,6 +1128,10 @@ typedef struct { >> uint32_t num_vmemranges; >> >> xc_domain_configuration_t config; >> +#if defined(__arm__) || defined(__aarch64__) >> + unsigned long vpl011_console_mfn; >> +#endif > > I am not a big fan of these #ifdef. > > Could they go away and this could would be compiled on x86 too but > just never used? (The default value to use this owuld be disabled > for example)? > >> } libxl__domain_build_state; >> >> _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid, >> -- >> 2.7.4 >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> https://lists.xen.org/xen-devel
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index d400fa2..cc00235 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -3159,6 +3159,12 @@ 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 defined(__arm__) || defined(__aarch64__) + flexarray_append(ro_front, "vpl011-port"); + flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vpl011_console_port)); + flexarray_append(ro_front, "vpl011-ring-ref"); + flexarray_append(ro_front, GCSPRINTF("%lu", state->vpl011_console_mfn)); +#endif } 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 d519c8d..39eaff6 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -302,7 +302,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, libxl_ctx *ctx = libxl__gc_owner(gc); char *xs_domid, *con_domid; int rc; - uint64_t size; + uint64_t size, val=-1; if (xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus) != 0) { LOG(ERROR, "Couldn't set max vcpu count"); @@ -432,6 +432,16 @@ 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 defined(__arm__) || defined(__aarch64__) + /* get the vpl011 event channel from Xen */ + rc = xc_hvm_param_get(ctx->xch, domid, HVM_PARAM_VPL011_CONSOLE_EVTCHN, + &val); + if ( rc ) + state->vpl011_console_port = -1; + else + state->vpl011_console_port = (uint32_t)val; +#endif + if (info->type == LIBXL_DOMAIN_TYPE_HVM) { hvm_set_conf_params(ctx->xch, domid, info); #if defined(__i386__) || defined(__x86_64__) @@ -727,6 +737,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, dom->flags = flags; dom->console_evtchn = state->console_port; +#if defined(__arm__) || defined(__aarch64__) + dom->vpl011_console_evtchn = state->vpl011_console_port; +#endif dom->console_domid = state->console_domid; dom->xenstore_evtchn = state->store_port; dom->xenstore_domid = state->store_domid; @@ -771,6 +784,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, if (xc_dom_feature_translated(dom)) { state->console_mfn = dom->console_pfn; state->store_mfn = dom->xenstore_pfn; +#if defined(__arm__) || defined(__aarch64__) + state->vpl011_console_mfn = dom->vpl011_console_pfn; +#endif } else { state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 5f46578..10e262e 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1128,6 +1128,10 @@ typedef struct { uint32_t num_vmemranges; xc_domain_configuration_t config; +#if defined(__arm__) || defined(__aarch64__) + unsigned long vpl011_console_mfn; + uint32_t vpl011_console_port; +#endif } libxl__domain_build_state; _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
Add two new parameters to the xen store: - newly allocated PFN to be used as IN/OUT ring buffer by xenconsoled - a new event channel read from Xen using a hvm call to be used by xenconsoled for eventing Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> --- tools/libxl/libxl.c | 6 ++++++ tools/libxl/libxl_dom.c | 18 +++++++++++++++++- tools/libxl/libxl_internal.h | 4 ++++ 3 files changed, 27 insertions(+), 1 deletion(-)