Re: [Xen-devel] [PATCH] add privileged/unprivileged kernel feature indication

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2011-07-05 at 13:48 +0100, Jan Beulich wrote:
> With our switching away from supporting 32-bit Dom0 operation, users
> complained that attempts (perhaps due to lack of knowledge of that
> change) to boot the no longer privileged kernel in Dom0 resulted in
> apparently silent failure. To make the mismatch explicit and visible,
> add feature flags that the kernel can set to indicate operation in
> what modes it supports. For backward compatibility, absence of both
> feature flags is taken to indicate a kernel that may be capable of
> operating in both modes.

While I agree that this is a useful change I think you should also try
and ensure that your bootloader configuration tool tries not to add
invalid combinations to the configuration. For example grub2's
"update-grub" command checks for CONFIG_XEN_PRIVILEGED_GUEST=y before
creating a dom0 style entry. IIRC grub1 did the same (although that may
have been Debian specific).

Ian.

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> --- a/tools/libxc/xc_dom_elfloader.c
> +++ b/tools/libxc/xc_dom_elfloader.c
> @@ -286,6 +286,15 @@ static int xc_dom_parse_elf_kernel(struc
>      if ( (rc = elf_xen_parse(elf, &dom->parms)) != 0 )
>          return rc;
>  
> +    if ( elf_xen_feature_get(XENFEAT_privileged, dom->parms.f_required) ||
> +         (elf_xen_feature_get(XENFEAT_privileged, dom->parms.f_supported) &&
> +          !elf_xen_feature_get(XENFEAT_unprivileged, 
> dom->parms.f_supported)) )
> +    {
> +        xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: Kernel does not"
> +                     " support unprivileged (DomU) operation", __FUNCTION__);
> +        return -EINVAL;
> +    }
> +
>      /* find kernel segment */
>      dom->kernel_seg.vstart = dom->parms.virt_kstart;
>      dom->kernel_seg.vend   = dom->parms.virt_kend;
> --- a/xen/arch/ia64/xen/domain.c
> +++ b/xen/arch/ia64/xen/domain.c
> @@ -2164,6 +2164,14 @@ int __init construct_dom0(struct domain 
>               return -1;
>       }
>  
> +     if (test_bit(XENFEAT_unprivileged, parms.f_required) ||
> +         (test_bit(XENFEAT_unprivileged, parms.f_supported) &&
> +          !test_bit(XENFEAT_privileged, parms.f_supported)))
> +     {
> +             printk("Kernel does not support Dom0 operation\n");
> +             return -1;
> +     }
> +
>       p_start = parms.virt_base;
>       pkern_start = parms.virt_kstart;
>       pkern_end = parms.virt_kend;
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -415,6 +415,14 @@ int __init construct_dom0(
>          return -EINVAL;
>      }
>  
> +    if ( test_bit(XENFEAT_unprivileged, parms.f_required) ||
> +         (test_bit(XENFEAT_unprivileged, parms.f_supported) &&
> +          !test_bit(XENFEAT_privileged, parms.f_supported)) )
> +    {
> +        printk("Kernel does not support Dom0 operation\n");
> +        return -EINVAL;
> +    }
> +
>  #if defined(__x86_64__)
>      if ( compat32 )
>      {
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -278,7 +278,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDL
>          switch ( fi.submap_idx )
>          {
>          case 0:
> -            fi.submap = 0;
> +            fi.submap = 1U << (IS_PRIV(current->domain) ?
> +                               XENFEAT_privileged : XENFEAT_unprivileged);
>              if ( VM_ASSIST(d, VMASST_TYPE_pae_extended_cr3) )
>                  fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb);
>              if ( paging_mode_translate(current->domain) )
> --- a/xen/common/libelf/libelf-dominfo.c
> +++ b/xen/common/libelf/libelf-dominfo.c
> @@ -26,7 +26,9 @@ static const char *const elf_xen_feature
>      [XENFEAT_writable_descriptor_tables] = "writable_descriptor_tables",
>      [XENFEAT_auto_translated_physmap] = "auto_translated_physmap",
>      [XENFEAT_supervisor_mode_kernel] = "supervisor_mode_kernel",
> -    [XENFEAT_pae_pgdir_above_4gb] = "pae_pgdir_above_4gb"
> +    [XENFEAT_pae_pgdir_above_4gb] = "pae_pgdir_above_4gb",
> +    [XENFEAT_privileged] = "privileged",
> +    [XENFEAT_unprivileged] = "unprivileged"
>  };
>  static const int elf_xen_features =
>  sizeof(elf_xen_feature_names) / sizeof(elf_xen_feature_names[0]);
> --- a/xen/include/public/features.h
> +++ b/xen/include/public/features.h
> @@ -75,7 +75,13 @@
>  #define XENFEAT_hvm_safe_pvclock           9
>  
>  /* x86: pirq can be used by HVM guests */
> -#define XENFEAT_hvm_pirqs           10
> +#define XENFEAT_hvm_pirqs                 10
> +
> +/* privileged operation is supported */
> +#define XENFEAT_privileged                11
> +
> +/* un-privileged operation is supported */
> +#define XENFEAT_unprivileged              12
>  
>  #define XENFEAT_NR_SUBMAPS 1
>  
> 
> 



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