[Xen-devel,04/12,v3] xen/arm: vpl011: Add support for vuart in libxl

Message ID 1494426481-32562-1-git-send-email-bhupinder.thakur@linaro.org
State New
Headers show
Series
  • Untitled series #1372
Related show

Commit Message

Bhupinder Thakur May 10, 2017, 2:28 p.m.
An option is provided in libxl to enable/disable pl011 vuart while
creating a guest domain.

Libxl now suppots a generic vuart console and pl011 is a specific type.
In future support can be added for multiple vuart of different types.

User can enable pl011 vuart by adding the following line in the guest
configuration file:

vuart = "pl011"

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---

Changes since v2:

- Defined vuart option as an enum instead of a string.
- Removed the domain creation flag defined for vuart and the related code
  to pass on the information while domain creation. Now vpl011 is initialized
  independent of domain creation through new DOMCTL APIs.

 tools/libxl/libxl.h          | 6 ++++++
 tools/libxl/libxl_console.c  | 3 +++
 tools/libxl/libxl_create.c   | 2 ++
 tools/libxl/libxl_internal.h | 3 +++
 tools/libxl/libxl_types.idl  | 7 +++++++
 tools/xl/xl_console.c        | 4 +++-
 tools/xl/xl_parse.c          | 8 ++++++++
 7 files changed, 32 insertions(+), 1 deletion(-)

Comments

Wei Liu May 11, 2017, 11:19 a.m. | #1
On Wed, May 10, 2017 at 07:58:01PM +0530, Bhupinder Thakur wrote:
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 856a304..504ca7c 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -916,6 +916,14 @@ void parse_config_data(const char *config_source,
>      if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
>          b_info->max_vcpus = l;
>  
> +    if (!xlu_cfg_get_string(config, "vuart", &buf, 0)) {
> +        if (libxl_vuart_type_from_string(buf, &b_info->vuart)) {
> +            fprintf(stderr, "ERROR: invalid value \"%s\" for \"vuart\"\n",
> +                    buf);
> +            exit (1);

Extraneous space.

With this fixed:

Acked-by: Wei Liu <wei.liu2@citrix.com>
Wei Liu May 16, 2017, 3:02 p.m. | #2
On Wed, May 10, 2017 at 07:58:01PM +0530, Bhupinder Thakur wrote:
> index 2204425..f5dc62c 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -105,6 +105,7 @@ libxl_console_type = Enumeration("console_type", [
>      (0, "UNKNOWN"),
>      (1, "SERIAL"),
>      (2, "PV"),
> +    (3, "VUART"),
>      ])
>  
>  libxl_disk_format = Enumeration("disk_format", [
> @@ -240,6 +241,11 @@ libxl_checkpointed_stream = Enumeration("checkpointed_stream", [
>      (2, "COLO"),
>      ])
>  
> +libxl_vuart_type = Enumeration("vuart_type", [
> +    (0, "unknown"),
> +    (1, "pl011"),
> +    ])
> +
>  #
>  # Complex libxl types
>  #
> @@ -470,6 +476,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>      ("disable_migrate", libxl_defbool),
>      ("cpuid",           libxl_cpuid_policy_list),
>      ("blkdev_start",    string),
> +    ("vuart",           libxl_vuart_type),
>  

It occurred to me you didn't put this into arch-specific build_info. I
need to withhold my ack. What is the reason for doing this?
Stefano Stabellini May 16, 2017, 10:46 p.m. | #3
On Wed, 10 May 2017, Bhupinder Thakur wrote:
> An option is provided in libxl to enable/disable pl011 vuart while
> creating a guest domain.
> 
> Libxl now suppots a generic vuart console and pl011 is a specific type.
> In future support can be added for multiple vuart of different types.
> 
> User can enable pl011 vuart by adding the following line in the guest
> configuration file:
> 
> vuart = "pl011"
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>

This looks OK to me.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> 
> Changes since v2:
> 
> - Defined vuart option as an enum instead of a string.
> - Removed the domain creation flag defined for vuart and the related code
>   to pass on the information while domain creation. Now vpl011 is initialized
>   independent of domain creation through new DOMCTL APIs.
> 
>  tools/libxl/libxl.h          | 6 ++++++
>  tools/libxl/libxl_console.c  | 3 +++
>  tools/libxl/libxl_create.c   | 2 ++
>  tools/libxl/libxl_internal.h | 3 +++
>  tools/libxl/libxl_types.idl  | 7 +++++++
>  tools/xl/xl_console.c        | 4 +++-
>  tools/xl/xl_parse.c          | 8 ++++++++
>  7 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index cf8687a..bcfbb6c 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -306,6 +306,12 @@
>  #define LIBXL_HAVE_BUILDINFO_HVM_ACPI_LAPTOP_SLATE 1
>  
>  /*
> + * LIBXL_HAVE_VUART indicates that xenconsole/client supports
> + * virtual uart.
> + */
> +#define LIBXL_HAVE_VUART 1
> +
> +/*
>   * libxl ABI compatibility
>   *
>   * The only guarantee which libxl makes regarding ABI compatibility
> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
> index 446e766..853be15 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;
>      }
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index bffbc45..29daa35 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -918,6 +918,8 @@ static void initiate_domain_create(libxl__egc *egc,
>          goto error_out;
>      }
>  
> +    state->config.console_domid = state->console_domid;
> +
>      ret = libxl__domain_make(gc, d_config, &domid, &state->config);
>      if (ret) {
>          LOGD(ERROR, domid, "cannot make domain: %d", ret);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 5d082c5..4e2c247 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1135,6 +1135,9 @@ typedef struct {
>      uint32_t num_vmemranges;
>  
>      xc_domain_configuration_t config;
> +
> +    xen_pfn_t vuart_gfn;
> +    evtchn_port_t vuart_port;
>  } libxl__domain_build_state;
>  
>  _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 2204425..f5dc62c 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -105,6 +105,7 @@ libxl_console_type = Enumeration("console_type", [
>      (0, "UNKNOWN"),
>      (1, "SERIAL"),
>      (2, "PV"),
> +    (3, "VUART"),
>      ])
>  
>  libxl_disk_format = Enumeration("disk_format", [
> @@ -240,6 +241,11 @@ libxl_checkpointed_stream = Enumeration("checkpointed_stream", [
>      (2, "COLO"),
>      ])
>  
> +libxl_vuart_type = Enumeration("vuart_type", [
> +    (0, "unknown"),
> +    (1, "pl011"),
> +    ])
> +
>  #
>  # Complex libxl types
>  #
> @@ -470,6 +476,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>      ("disable_migrate", libxl_defbool),
>      ("cpuid",           libxl_cpuid_policy_list),
>      ("blkdev_start",    string),
> +    ("vuart",           libxl_vuart_type),
>  
>      ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")),
>      
> diff --git a/tools/xl/xl_console.c b/tools/xl/xl_console.c
> index 0508dda..6f3cd7f 100644
> --- a/tools/xl/xl_console.c
> +++ b/tools/xl/xl_console.c
> @@ -34,8 +34,10 @@ int main_console(int argc, char **argv)
>              type = LIBXL_CONSOLE_TYPE_PV;
>          else if (!strcmp(optarg, "serial"))
>              type = LIBXL_CONSOLE_TYPE_SERIAL;
> +        else if (!strcmp(optarg, "vuart"))
> +            type = LIBXL_CONSOLE_TYPE_VUART;
>          else {
> -            fprintf(stderr, "console type supported are: pv, serial\n");
> +            fprintf(stderr, "console type supported are: pv, serial, vuart\n");
>              return EXIT_FAILURE;
>          }
>          break;
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 856a304..504ca7c 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -916,6 +916,14 @@ void parse_config_data(const char *config_source,
>      if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
>          b_info->max_vcpus = l;
>  
> +    if (!xlu_cfg_get_string(config, "vuart", &buf, 0)) {
> +        if (libxl_vuart_type_from_string(buf, &b_info->vuart)) {
> +            fprintf(stderr, "ERROR: invalid value \"%s\" for \"vuart\"\n",
> +                    buf);
> +            exit (1);
> +        }
> +    }
> +
>      parse_vnuma_config(config, b_info);
>  
>      /* Set max_memkb to target_memkb and max_vcpus to avail_vcpus if
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
Bhupinder Thakur May 23, 2017, 7:35 a.m. | #4
Hi Wei,

>>  # Complex libxl types
>>  #
>> @@ -470,6 +476,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>>      ("disable_migrate", libxl_defbool),
>>      ("cpuid",           libxl_cpuid_policy_list),
>>      ("blkdev_start",    string),
>> +    ("vuart",           libxl_vuart_type),
>>
>
> It occurred to me you didn't put this into arch-specific build_info. I
> need to withhold my ack. What is the reason for doing this?

I will move the definition to arch-arm specific part of build_info.

Regards,
Bhupinder

Patch hide | download patch | download mbox

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index cf8687a..bcfbb6c 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -306,6 +306,12 @@ 
 #define LIBXL_HAVE_BUILDINFO_HVM_ACPI_LAPTOP_SLATE 1
 
 /*
+ * LIBXL_HAVE_VUART indicates that xenconsole/client supports
+ * virtual uart.
+ */
+#define LIBXL_HAVE_VUART 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 446e766..853be15 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;
     }
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bffbc45..29daa35 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -918,6 +918,8 @@  static void initiate_domain_create(libxl__egc *egc,
         goto error_out;
     }
 
+    state->config.console_domid = state->console_domid;
+
     ret = libxl__domain_make(gc, d_config, &domid, &state->config);
     if (ret) {
         LOGD(ERROR, domid, "cannot make domain: %d", ret);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5d082c5..4e2c247 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1135,6 +1135,9 @@  typedef struct {
     uint32_t num_vmemranges;
 
     xc_domain_configuration_t config;
+
+    xen_pfn_t vuart_gfn;
+    evtchn_port_t vuart_port;
 } libxl__domain_build_state;
 
 _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 2204425..f5dc62c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -105,6 +105,7 @@  libxl_console_type = Enumeration("console_type", [
     (0, "UNKNOWN"),
     (1, "SERIAL"),
     (2, "PV"),
+    (3, "VUART"),
     ])
 
 libxl_disk_format = Enumeration("disk_format", [
@@ -240,6 +241,11 @@  libxl_checkpointed_stream = Enumeration("checkpointed_stream", [
     (2, "COLO"),
     ])
 
+libxl_vuart_type = Enumeration("vuart_type", [
+    (0, "unknown"),
+    (1, "pl011"),
+    ])
+
 #
 # Complex libxl types
 #
@@ -470,6 +476,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
     ("disable_migrate", libxl_defbool),
     ("cpuid",           libxl_cpuid_policy_list),
     ("blkdev_start",    string),
+    ("vuart",           libxl_vuart_type),
 
     ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")),
     
diff --git a/tools/xl/xl_console.c b/tools/xl/xl_console.c
index 0508dda..6f3cd7f 100644
--- a/tools/xl/xl_console.c
+++ b/tools/xl/xl_console.c
@@ -34,8 +34,10 @@  int main_console(int argc, char **argv)
             type = LIBXL_CONSOLE_TYPE_PV;
         else if (!strcmp(optarg, "serial"))
             type = LIBXL_CONSOLE_TYPE_SERIAL;
+        else if (!strcmp(optarg, "vuart"))
+            type = LIBXL_CONSOLE_TYPE_VUART;
         else {
-            fprintf(stderr, "console type supported are: pv, serial\n");
+            fprintf(stderr, "console type supported are: pv, serial, vuart\n");
             return EXIT_FAILURE;
         }
         break;
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 856a304..504ca7c 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -916,6 +916,14 @@  void parse_config_data(const char *config_source,
     if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
         b_info->max_vcpus = l;
 
+    if (!xlu_cfg_get_string(config, "vuart", &buf, 0)) {
+        if (libxl_vuart_type_from_string(buf, &b_info->vuart)) {
+            fprintf(stderr, "ERROR: invalid value \"%s\" for \"vuart\"\n",
+                    buf);
+            exit (1);
+        }
+    }
+
     parse_vnuma_config(config, b_info);
 
     /* Set max_memkb to target_memkb and max_vcpus to avail_vcpus if