All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] add privileged/unprivileged kernel feature indication
@ 2011-07-05 12:48 Jan Beulich
  2011-07-05 13:34 ` Jan Beulich
  2011-07-05 13:55 ` Ian Campbell
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2011-07-05 12:48 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: Type: text/plain, Size: 4107 bytes --]

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.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- 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
 



[-- Attachment #2: guest-kernel-cap.patch --]
[-- Type: text/plain, Size: 4103 bytes --]

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.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- 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
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] add privileged/unprivileged kernel feature indication
  2011-07-05 12:48 [PATCH] add privileged/unprivileged kernel feature indication Jan Beulich
@ 2011-07-05 13:34 ` Jan Beulich
  2011-07-05 13:55 ` Ian Campbell
  1 sibling, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2011-07-05 13:34 UTC (permalink / raw)
  To: xen-devel

>>> On 05.07.11 at 14:48, "Jan Beulich" <JBeulich@novell.com> 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.

With elf_xen_parse_features() erroring out upon encountering
an unrecognized feature, the only alternative to this approach
that I can see would be to add a new note type, and store the
information there. With that, it would seem to make sense to
store all required/supported features in binary form, making the
parsing unnecessary on hypervisors that understand this new
note type.

Does anyone have any other ideas?

Should this erroring out be fixed (to only do so on unrecognized
*required* features) at least for future releases?

Jan

> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- 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
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] add privileged/unprivileged kernel feature indication
  2011-07-05 12:48 [PATCH] add privileged/unprivileged kernel feature indication Jan Beulich
  2011-07-05 13:34 ` Jan Beulich
@ 2011-07-05 13:55 ` Ian Campbell
  2011-07-05 14:15   ` Jan Beulich
  2011-07-08 17:37   ` Ian Jackson
  1 sibling, 2 replies; 6+ messages in thread
From: Ian Campbell @ 2011-07-05 13:55 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

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@novell.com>
> 
> --- 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
>  
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] add privileged/unprivileged kernel feature indication
  2011-07-05 13:55 ` Ian Campbell
@ 2011-07-05 14:15   ` Jan Beulich
  2011-07-05 14:27     ` Ian Campbell
  2011-07-08 17:37   ` Ian Jackson
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2011-07-05 14:15 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

>>> On 05.07.11 at 15:55, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 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).

That I would consider completely bogus - the boot loader should not
have a need to know *anything* about the kind of kernel it boots.

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] add privileged/unprivileged kernel  feature indication
  2011-07-05 14:15   ` Jan Beulich
@ 2011-07-05 14:27     ` Ian Campbell
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2011-07-05 14:27 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

On Tue, 2011-07-05 at 15:15 +0100, Jan Beulich wrote:
> >>> On 05.07.11 at 15:55, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > 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).
> 
> That I would consider completely bogus - the boot loader should not
> have a need to know *anything* about the kind of kernel it boots.

I said bootloader configuration tool (i.e. the thing which helps users
generate /boot/grub/grub.cfg), not the bootloader itself. i.e. the tool
should not be adding xen+domU-only-kernel as an option in grub.cfg.

If this tool doesn't understand these things you end up presenting users
with booloader options which cannot ever work and then they get
confused.

Ian.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] add privileged/unprivileged kernel feature indication
  2011-07-05 13:55 ` Ian Campbell
  2011-07-05 14:15   ` Jan Beulich
@ 2011-07-08 17:37   ` Ian Jackson
  1 sibling, 0 replies; 6+ messages in thread
From: Ian Jackson @ 2011-07-08 17:37 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Jan Beulich

Ian Campbell writes ("Re: [Xen-devel] [PATCH] add privileged/unprivileged kernel feature indication"):
> 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).

Quite so.

(On this subject, Debian bug #633127 may be relevant to people using
squeeze or other grub2-based systems with "traditional xen" kernels.)

Ian.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-07-08 17:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-05 12:48 [PATCH] add privileged/unprivileged kernel feature indication Jan Beulich
2011-07-05 13:34 ` Jan Beulich
2011-07-05 13:55 ` Ian Campbell
2011-07-05 14:15   ` Jan Beulich
2011-07-05 14:27     ` Ian Campbell
2011-07-08 17:37   ` Ian Jackson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.