WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xen: update machine_to_phys_order on resume

>>> On 18.07.11 at 10:31, Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> Pushing things down at build time is pretty easy. FWIW here's an
> incomplete patch to push the kernel declared features down into the
> hypervisor at build time extracted from some old PV in HVM container
> stuff (so not directly applicable). I can bring it up to date if the
> approach seems useful.

Yes, that looks like what we'd want from a conceptual perspective,
but...

> One thing which is somewhat missing is user control of non-mandatory
> features declared by a kernel, although normally that should be a
> decision made by the tools/hypervisor based upon available features etc.
> 
> diff -r a6c4d03b7d45 tools/libxc/xc_dom_core.c
> --- a/tools/libxc/xc_dom_core.c       Mon Feb 08 13:05:48 2010 +0000
> +++ b/tools/libxc/xc_dom_core.c       Thu Feb 11 12:48:47 2010 +0000
> @@ -609,6 +609,7 @@
>  
>  int xc_dom_parse_image(struct xc_dom_image *dom)
>  {
> +    DECLARE_DOMCTL;
>      int i;
>  
>      xc_dom_printf("%s: called\n", __FUNCTION__);
> @@ -629,8 +630,26 @@
>      /* check features */
>      for ( i = 0; i < XENFEAT_NR_SUBMAPS; i++ )
>      {
> -        dom->f_active[i] |= dom->f_requested[i]; /* cmd line */
> -        dom->f_active[i] |= dom->parms.f_required[i]; /* kernel   */
> +        domctl.cmd = XEN_DOMCTL_setfeatures;
> +        domctl.domain = dom->guest_domid;
> +
> +        domctl.u.setfeatures.submap_idx = i;
> +        domctl.u.setfeatures.submap = 0;
> +
> +        domctl.u.setfeatures.submap |= dom->f_requested[i]; /* cmd line */
> +        domctl.u.setfeatures.submap |= dom->parms.f_required[i]; /* kernel   
> */
> +
> +        xc_dom_printf("requesting features[%d] = %#x\n", 
> domctl.u.setfeatures.submap_idx, domctl.u.setfeatures.submap);
> +        if (do_domctl(dom->guest_xc, &domctl))
> +        {
> +            xc_dom_panic(XC_INVALID_PARAM,
> +                         "%s: unable to set requested features\n", 
> __FUNCTION__);
> +            goto err;
> +        }
> +
> +        xc_dom_printf("received   features[%d] = %#x\n", 
> domctl.u.setfeatures.submap_idx, domctl.u.setfeatures.submap);
> +        dom->f_active[i] = domctl.u.setfeatures.submap;
> +
>          if ( (dom->f_active[i] & dom->parms.f_supported[i]) !=
>               dom->f_active[i] )
>          {
> @@ -639,6 +658,7 @@
>              goto err;
>          }
>      }
> +
>      return 0;
>  
>   err:
> diff -r a6c4d03b7d45 tools/libxl/xl.c
> --- a/tools/libxl/xl.c        Mon Feb 08 13:05:48 2010 +0000
> +++ b/tools/libxl/xl.c        Thu Feb 11 12:48:47 2010 +0000
> @@ -468,6 +468,8 @@
>          }
>          if (config_lookup_string (&config, "ramdisk", &buf) == CONFIG_TRUE)
>              b_info->u.pv.ramdisk = strdup(buf);
> +        if (config_lookup_string (&config, "features", &buf) == CONFIG_TRUE)
> +            b_info->u.pv.features = strdup(buf);
>      }
>  
>      if ((vbds = config_lookup (&config, "disk")) != NULL) {
> diff -r a6c4d03b7d45 xen/common/domctl.c
> --- a/xen/common/domctl.c     Mon Feb 08 13:05:48 2010 +0000
> +++ b/xen/common/domctl.c     Thu Feb 11 12:48:47 2010 +0000
> @@ -23,6 +23,7 @@
>  #include <xen/paging.h>
>  #include <asm/current.h>
>  #include <public/domctl.h>
> +#include <public/features.h>
>  #include <xsm/xsm.h>
>  
>  static DEFINE_SPINLOCK(domctl_lock);
> @@ -960,6 +962,54 @@
>      }
>      break;
>  
> +    case XEN_DOMCTL_setfeatures:
> +    {
> +        struct domain *d;
> +        ret = -ESRCH;
> +        if ( (d = rcu_lock_domain_by_id(op->domain)) != NULL )
> +        {
> +            printk("dom%d set features[%d] = %#x\n", d->domain_id, 
> op->u.setfeatures.submap_idx, op->u.setfeatures.submap);
> +
> +            switch (op->u.setfeatures.submap_idx) {
> +            case 0:
> +                if ( !paging_mode_translate(d) )

... this condition looks inverted to me.

> +                {
> +                    op->u.setfeatures.submap &= 
> ~(1U<<XENFEAT_writable_page_tables);
> +                    op->u.setfeatures.submap &= 
> ~(1U<<XENFEAT_auto_translated_physmap);
> +                }
> +                if ( !is_pvhvm_domain(d) )
> +                {
> +                    op->u.setfeatures.submap &= 
> ~(1U<<XENFEAT_supervisor_mode_kernel);
> +                }
> +
> +                op->u.setfeatures.submap &= 
> ~(1U<<XENFEAT_writable_descriptor_tables);

Why do you turn this off unconditionally?

> +
> +                /* XXX other features */

That's perhaps also the place holder where the passed in information
would actually get stored?

Jan

> +
> +
> +                if ( op->u.setfeatures.submap 
> &(1U<<XENFEAT_supervisor_mode_kernel) )
> +                    d->arch.pv_kernel_minimum_rpl = 0;
> +
> +                ret = 0;
> +                break;
> +
> +            default:
> +                printk("dom%d unknown feature submap %d\n", d->domain_id, 
> op->u.setfeatures.submap_idx);
> +                op->u.setfeatures.submap = 0;
> +                ret = -EINVAL;
> +                break;
> +            }
> +
> +            rcu_unlock_domain(d);
> +            ret = 0;
> +
> +            if ( copy_to_guest(u_domctl, op, 1) )
> +                ret = -EFAULT;
> +
> +        }
> +    }
> +    break;
> +
>      default:
>          ret = arch_do_domctl(op, u_domctl);
>          break;
> diff -r a6c4d03b7d45 xen/include/public/domctl.h
> --- a/xen/include/public/domctl.h     Mon Feb 08 13:05:48 2010 +0000
> +++ b/xen/include/public/domctl.h     Thu Feb 11 12:48:47 2010 +0000
> @@ -169,6 +169,13 @@
>      XEN_GUEST_HANDLE_64(xen_pfn_t) array;
>  };
>  
> +/* XEN_DOMCTL_setfeatures */
> +struct xen_domctl_setfeatures {
> +    /* IN variables */
> +    unsigned int submap_idx;    /* which 32-bit submap to return */
> +    /* IN/OUT variables */
> +    uint32_t     submap;        /* 32-bit submap, updated with actual 
> result. */
> +};
>  
>  /*
>   * Control shadow pagetables operation
> @@ -842,6 +848,7 @@
>  #define XEN_DOMCTL_gettscinfo                    59
>  #define XEN_DOMCTL_settscinfo                    60
>  #define XEN_DOMCTL_getpageframeinfo3             61
> +#define XEN_DOMCTL_setfeatures                        62
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -855,6 +862,7 @@
>          struct xen_domctl_getpageframeinfo  getpageframeinfo;
>          struct xen_domctl_getpageframeinfo2 getpageframeinfo2;
>          struct xen_domctl_getpageframeinfo3 getpageframeinfo3;
> +        struct xen_domctl_setfeatures                setfeatures;
>          struct xen_domctl_vcpuaffinity      vcpuaffinity;
>          struct xen_domctl_shadow_op         shadow_op;
>          struct xen_domctl_max_mem           max_mem;



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