Re: [Xen-devel] [PATCH] libxl: use named options for tsc_mode

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2011-11-10 at 10:55 +0000, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1320922479 0
> # Node ID bc79b560aafa1e4dc42af00e6a326dc651b5636a
> # Parent  460b507e15f864dd6712f5040e36538d6e076ae4
> libxl: use named options for tsc_mode.
> 
> It seems that this knob is expoerted from the hypervisor as a raw
> integer (no symbolic names) documented in xen/include/asm-x86. Propagating 
> that
> all the way to the end user is hardly friendly (it's bad enough in the
> hypercall interface).

I've just seen docs/misc/tscmode.txt which is actually a pretty
comprehensive explanation of what's going on so the above is
unwarranted. I'll update this patch to include a reference in the
xl-domain-config.

> 
> Add an enum at the libxl level with a hopefully descriptive set of names.
> Deprecate the use of an integer in xl cfg files.

I think the naming at the libxl level is still useful even if the values
are documented.

Ian.

> 
> Signed-off-by: Ian Campbell
> 
> diff -r 460b507e15f8 -r bc79b560aafa docs/user/xl-domain-config.markdown
> --- a/docs/user/xl-domain-config.markdown     Thu Nov 10 10:18:29 2011 +0000
> +++ b/docs/user/xl-domain-config.markdown     Thu Nov 10 10:54:39 2011 +0000
> @@ -1,4 +1,4 @@
> - # xl Domain Configuration
> +# xl Domain Configuration
>  
>  To create a VM (a domain in Xen terminology, sometimes called a guest)
>  with xl requires the provision of a domain config file.  Typically
> @@ -338,6 +338,29 @@ accept the defaults for these options wh
>     extensions (e.g. Windows XP compatibility mode on more modern
>     Windows OS).
>  
> +### Guest Virtual Time Controls
> +
> + * `tsc_mode="MODE"`: Specifies how the TSC (Time Stamp Counter)
> +   should be provided to the guest (X86 only). Specifying this option as a 
> number
> +   is deprecated. Options are:
> +
> +   * `"default"`: guest rdtsc/p executed natively when monotonicity
> +     can be guaranteed and emulated otherwise (with frequency scaled
> +     if necessary).
> +
> +   * `"always_emulate"`: guest rdtsc/p always emulated at 1GHz (kernel
> +     and user).
> +
> +   * `"native"`: guest rdtsc always executed natively (no
> +     monotonicity/frequency guarantees); guest rdtscp emulated at
> +     native frequency if unsupported by h/w, else executed natively.
> +
> +   * `"native_paravirt"`: same as `native`, except xen manages TSC_AUX
> +      register so guest can determine when a restore/migration has
> +      occurred and assumes guest obtains/uses pvclock-like mechanism
> +      to adjust for monotonicity and frequency changes.
> +
> +
>  ### Support for Paravirtualisation of HVM Guests
>  
>  The following options allow Paravirtualised features (such as devices)
> @@ -546,9 +569,6 @@ certainly belong in a more appropriate s
>     enables certain other features which are incompatible with
>     migration (currently certain TSC modes XXX ?).
>  
> - * `tsc_mode=VALUE`: Specifies how the TSC (Time Stamp Counter) should
> -   be provided to the guest.  XXX ???
> -
>   * `pci_msitranslate=BOOLEAN`: XXX
>  
>   * `pci_power_mgmt=BOOLEAN`: XXX
> diff -r 460b507e15f8 -r bc79b560aafa tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c Thu Nov 10 10:18:29 2011 +0000
> +++ b/tools/libxl/libxl_dom.c Thu Nov 10 10:54:39 2011 +0000
> @@ -73,12 +73,29 @@ int libxl__build_pre(libxl__gc *gc, uint
>                libxl_domain_build_info *info, libxl__domain_build_state 
> *state)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
> +    int tsc_mode;
>      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
>      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + 
> LIBXL_MAXMEM_CONSTANT);
>      if (info->type == LIBXL_DOMAIN_TYPE_PV)
>          xc_domain_set_memmap_limit(ctx->xch, domid,
>                  (info->max_memkb + info->u.pv.slack_memkb));
> -    xc_domain_set_tsc_info(ctx->xch, domid, info->tsc_mode, 0, 0, 0);
> +    switch (info->tsc_mode) {
> +    case LIBXL_TSC_MODE_DEFAULT:
> +        tsc_mode = 0;
> +        break;
> +    case LIBXL_TSC_MODE_ALWAYS_EMULATE:
> +        tsc_mode = 1;
> +        break;
> +    case LIBXL_TSC_MODE_NATIVE:
> +        tsc_mode = 2;
> +        break;
> +    case LIBXL_TSC_MODE_NATIVE_PARAVIRT:
> +        tsc_mode = 3;
> +        break;
> +    default:
> +        abort();
> +    }
> +    xc_domain_set_tsc_info(ctx->xch, domid, tsc_mode, 0, 0, 0);
>      if ( info->disable_migrate )
>          xc_domain_disable_migrate(ctx->xch, domid);
>  
> diff -r 460b507e15f8 -r bc79b560aafa tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl     Thu Nov 10 10:18:29 2011 +0000
> +++ b/tools/libxl/libxl_types.idl     Thu Nov 10 10:54:39 2011 +0000
> @@ -85,6 +85,13 @@ libxl_button = Enumeration("button", [
>      (2, "SLEEP"),
>      ])
>  
> +libxl_tsc_mode = Enumeration("tsc_mode", [
> +    (0, "default"),
> +    (1, "always_emulate"),
> +    (2, "native"),
> +    (3, "native_paravirt"),
> +    ])
> +
>  #
>  # Complex libxl types
>  #
> @@ -154,7 +161,7 @@ libxl_domain_create_info = Struct("domai
>  libxl_domain_build_info = Struct("domain_build_info",[
>      ("max_vcpus",       integer),
>      ("cur_vcpus",       integer),
> -    ("tsc_mode",        integer),
> +    ("tsc_mode",        libxl_tsc_mode),
>      ("max_memkb",       uint32),
>      ("target_memkb",    uint32),
>      ("video_memkb",     uint32),
> diff -r 460b507e15f8 -r bc79b560aafa tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Thu Nov 10 10:18:29 2011 +0000
> +++ b/tools/libxl/xl_cmdimpl.c        Thu Nov 10 10:54:39 2011 +0000
> @@ -328,7 +328,7 @@ static void printf_info(int domid,
>  
>      printf("\t(build_info)\n");
>      printf("\t(max_vcpus %d)\n", b_info->max_vcpus);
> -    printf("\t(tsc_mode %d)\n", b_info->tsc_mode);
> +    printf("\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
>      printf("\t(max_memkb %d)\n", b_info->max_memkb);
>      printf("\t(target_memkb %d)\n", b_info->target_memkb);
>      printf("\t(nomigrate %d)\n", b_info->disable_migrate);
> @@ -662,8 +662,23 @@ static void parse_config_data(const char
>      if (!xlu_cfg_get_long (config, "nomigrate", &l, 0))
>          b_info->disable_migrate = l;
>  
> -    if (!xlu_cfg_get_long(config, "tsc_mode", &l, 0))
> +    if (!xlu_cfg_get_long(config, "tsc_mode", &l, 1)) {
> +        fprintf(stderr, "WARNING: specifying \"tsc_mode\" as an integer is 
> deprecated. "
> +                "Please use the named parameter variant.\n");
> +        if (l < LIBXL_TSC_MODE_DEFAULT ||
> +            l > LIBXL_TSC_MODE_NATIVE_PARAVIRT) {
> +            fprintf(stderr, "ERROR: invalid value %ld for \"tsc_mode\"\n", 
> l);
> +            exit (1);
> +        }
>          b_info->tsc_mode = l;
> +    } else if (!xlu_cfg_get_string(config, "tsc_mode", &buf, 0)) {
> +        fprintf(stderr, "got a tsc mode string: \"%s\"\n", buf);
> +        if (libxl_tsc_mode_from_string(buf, &b_info->tsc_mode)) {
> +            fprintf(stderr, "ERROR: invalid value \"%s\" for \"tsc_mode\"\n",
> +                    buf);
> +            exit (1);
> +        }
> +    }
>  
>      if (!xlu_cfg_get_long (config, "videoram", &l, 0))
>          b_info->video_memkb = l * 1024;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel