[Xen-devel,07/11] xen/arm: vpl011: Add two new vpl011 parameters to xenstore

Message ID 1487676368-22356-8-git-send-email-bhupinder.thakur@linaro.org
State New
Headers show
Series
  • pl011 emulation support in Xen
Related show

Commit Message

Bhupinder Thakur Feb. 21, 2017, 11:26 a.m.
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(-)

Comments

Konrad Rzeszutek Wilk March 3, 2017, 8:58 p.m. | #1
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

Patch hide | download patch | download mbox

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,