Xen devel - Jul 2011 - [PATCH, v2] add privileged/unprivileged kernel feature indication

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.

v2: Due to the way elf_xen_parse_features() worked up to now (getting
fixed here), adding features indications to the old, string based ELF
note would make the respective kernel unusable on older hypervisors.
For that reason, a new ELF Note is being introduced that allows
specifying supported features as a bit array instead (with features
unknown to the hypervisor simply ignored, as now also done by
elf_xen_parse_features(), whereas here unknown kernel-required
features still keep the kernel [and hence VM] from booting).

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]);
@@ -83,7 +85,7 @@ int elf_xen_parse_features(const char *f
                 }
             }
         }
-        if ( i == elf_xen_features )
+        if ( i == elf_xen_features && required && feature[0] ==
''!'' )
             return -1;
     }
 
@@ -114,6 +116,7 @@ int elf_xen_parse_note(struct elf_binary
         [XEN_ELFNOTE_LOADER] = { "LOADER", 1},
         [XEN_ELFNOTE_PAE_MODE] = { "PAE_MODE", 1},
         [XEN_ELFNOTE_FEATURES] = { "FEATURES", 1},
+        [XEN_ELFNOTE_SUPPORTED_FEATURES] = { "SUPPORTED_FEATURES",
0},
         [XEN_ELFNOTE_BSD_SYMTAB] = { "BSD_SYMTAB", 1},
         [XEN_ELFNOTE_SUSPEND_CANCEL] = { "SUSPEND_CANCEL", 0 },
         [XEN_ELFNOTE_MOD_START_PFN] = { "MOD_START_PFN", 0 },
@@ -122,6 +125,7 @@ int elf_xen_parse_note(struct elf_binary
 
     const char *str = NULL;
     uint64_t val = 0;
+    unsigned int i;
     int type = elf_uval(elf, note, type);
 
     if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
@@ -200,6 +204,14 @@ int elf_xen_parse_note(struct elf_binary
             return -1;
         break;
 
+    case XEN_ELFNOTE_SUPPORTED_FEATURES:
+        for ( i = 0; i < XENFEAT_NR_SUBMAPS; ++i )
+        {
+            parms->f_supported[i] |= val;
+            val >>= 32;
+        }
+        break;
+
     }
     return 0;
 }
--- a/xen/include/public/elfnote.h
+++ b/xen/include/public/elfnote.h
@@ -179,9 +179,22 @@
 #define XEN_ELFNOTE_MOD_START_PFN 16
 
 /*
+ * The features supported by this kernel (numeric).
+ *
+ * Other than XEN_ELFNOTE_FEATURES on pre-4.2 Xen, this note allows a
+ * kernel to specify support for features that older hypervisors don''t
+ * know about. The set of features 4.2 and newer hypervisors will
+ * consider supported by the kernel is the combination of the sets
+ * specified through this and the string note.
+ *
+ * LEGACY: FEATURES
+ */
+#define XEN_ELFNOTE_SUPPORTED_FEATURES 17
+
+/*
  * The number of the highest elfnote defined.
  */
-#define XEN_ELFNOTE_MAX XEN_ELFNOTE_MOD_START_PFN
+#define XEN_ELFNOTE_MAX XEN_ELFNOTE_SUPPORTED_FEATURES
 
 /*
  * System information exported through crash notes.
--- 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@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2011-07-19 at 10:00 +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.
> 
> v2: Due to the way elf_xen_parse_features() worked up to now (getting
> fixed here), adding features indications to the old, string based ELF
> note would make the respective kernel unusable on older hypervisors.
What was the failure mode? Can we not fix it (with suitable backport
recommendations) rather than adding a new duplicated interface?
> For that reason, a new ELF Note is being introduced that allows
> specifying supported features as a bit array instead (with features
> unknown to the hypervisor simply ignored, as now also done by
> elf_xen_parse_features(), whereas here unknown kernel-required
> features still keep the kernel [and hence VM] from booting).
 
> +    case XEN_ELFNOTE_SUPPORTED_FEATURES:
> +        for ( i = 0; i < XENFEAT_NR_SUBMAPS; ++i )
There needs to be some negotiation of what the kernel thought
XENFEAT_NR_SUBMAPS was or else if/when we have enough features to bump
the number we risk running off the end of the array on older kernels.
> +        {
> +            parms->f_supported[i] |= val;
val is a uint64_t so we don''t support more than two submaps, which is
ok
for now but the elf note needs to include a way to grow beyond that in a
forward and backward compatible way (lest we grow a third interface for
this in the future!).

Notes have a length field and we support 1,2 and 4 byte numerical notes
but here I think we need to add support for arbitrary length arrays on
numerical values.
>  /* 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
This still strikes me as odd because unprivileged is a subset of
privileged (I understand the backwards compatibility argument for having
it this way though). Really XENFEAT_unprivileged is the
"XENFEAT_privileged feature bit is supported" meta-feature flag.

Perhaps this should be a separate elf note? If present then it is 0 or 1
to indicate support for running privileged and if absent we assume it is
supported. This would also remove the need for:
> @@ -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);
Which information which is already exposed to the guest via start_info.

Do we really mean "privileged" here, or do we mean "dom0", I
know the
two are tied together today but will this be the case as we disaggregate
more and more? Does this flag really mean "can drive APICs and run ACPI
code etc"? Which is distinct from the ability to drive hardware
generally etc.

Ian.


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

>>> On 19.07.11 at 11:28, Ian Campbell <Ian.Campbell@citrix.com>
wrote:
> On Tue, 2011-07-19 at 10:00 +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.
>> 
>> v2: Due to the way elf_xen_parse_features() worked up to now (getting
>> fixed here), adding features indications to the old, string based ELF
>> note would make the respective kernel unusable on older hypervisors.
> 
> What was the failure mode? Can we not fix it (with suitable backport
> recommendations) rather than adding a new duplicated interface?
Adding a supported feature Xen doesn''t understand leads to a
"cannot load Dom0 kernel" without any indication what was actually
wrong with the kernel.

The fix is trivial (this patch''s change to elf_xen_parse_features()),
but expecting everyone to backport this to (perhaps very) old
hypervisors didn''t seem realistic to me.
>> For that reason, a new ELF Note is being introduced that allows
>> specifying supported features as a bit array instead (with features
>> unknown to the hypervisor simply ignored, as now also done by
>> elf_xen_parse_features(), whereas here unknown kernel-required
>> features still keep the kernel [and hence VM] from booting).
>  
>> +    case XEN_ELFNOTE_SUPPORTED_FEATURES:
>> +        for ( i = 0; i < XENFEAT_NR_SUBMAPS; ++i )
> 
> There needs to be some negotiation of what the kernel thought
> XENFEAT_NR_SUBMAPS was or else if/when we have enough features to bump
> the number we risk running off the end of the array on older kernels.
No. The kernel simply specifies what it''s (approximate) notion
through the note''s data size. Xen just reads as much as it
understands (and ignores, as written in the description) the rest.
>> +        {
>> +            parms->f_supported[i] |= val;
> 
> val is a uint64_t so we don''t support more than two submaps, which
is ok
> for now but the elf note needs to include a way to grow beyond that in a
> forward and backward compatible way (lest we grow a third interface for
> this in the future!).
Hmm, indeed - we''d have to improve elf_note_numeric() to be
forward compatible.
> Notes have a length field and we support 1,2 and 4 byte numerical notes
> but here I think we need to add support for arbitrary length arrays on
> numerical values.
Easiest would seem to be to have the caller (optionally) specify a unit
size and index. What do you think?
>>  /* 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
> 
> This still strikes me as odd because unprivileged is a subset of
> privileged (I understand the backwards compatibility argument for having
> it this way though). Really XENFEAT_unprivileged is the
> "XENFEAT_privileged feature bit is supported" meta-feature flag.
No, I don''t view it that way - in the Linux ports, the meaning of
the respective config options is such that privileged includes
unprivileged, but that''s a guest OS decision, not one the interface
should dictate.

The only implication done here is that the (otherwise meaningless)
absence of both flags gets taken as if both flags were set.
> Perhaps this should be a separate elf note? If present then it is 0 or 1
> to indicate support for running privileged and if absent we assume it is
> supported. This would also remove the need for:
> 
>> @@ -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);
> 
> Which information which is already exposed to the guest via start_info.
Yeah, I just wanted this for completeness. If it''s deemed bad to do so,
it could simply be dropped.
> Do we really mean "privileged" here, or do we mean
"dom0", I know the
> two are tied together today but will this be the case as we disaggregate
> more and more? Does this flag really mean "can drive APICs and run
ACPI
> code etc"? Which is distinct from the ability to drive hardware
> generally etc.
No, this is really only meaningful as a Dom0-capability indication in
today''s sense. Splitting this will probably yield the whole privileged/
unprivileged distinction bogus, and hence kernels supporting this
would probably be required to just always set both flags.

If the feature naming is a problem, we could certainly rename them
into "dom0" and "domU", to distinguish the two ways of
getting
loaded.

Jan

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

On Tue, 2011-07-19 at 11:24 +0100, Jan Beulich wrote:
> >>> On 19.07.11 at 11:28, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
> > On Tue, 2011-07-19 at 10:00 +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.
> >> 
> >> v2: Due to the way elf_xen_parse_features() worked up to now
(getting
> >> fixed here), adding features indications to the old, string based
ELF
> >> note would make the respective kernel unusable on older
hypervisors.
> > 
> > What was the failure mode? Can we not fix it (with suitable backport
> > recommendations) rather than adding a new duplicated interface?
> 
> Adding a supported feature Xen doesn''t understand leads to a
> "cannot load Dom0 kernel" without any indication what was
actually
> wrong with the kernel.
> 
> The fix is trivial (this patch''s change to
elf_xen_parse_features()),
> but expecting everyone to backport this to (perhaps very) old
> hypervisors didn''t seem realistic to me.
Backporting a trivial fix like this is pretty easy, especially to this
code which has been reasonably static for a long time.

People who need this fix are either building a modern kernel themselves
(in which case I expect they can cope with getting a fixed hypervisor
too) or they are getting it from their distro and then the packages
dependencies will cause the necessary hypervisor fix to get pulled in.
> >> +        {
> >> +            parms->f_supported[i] |= val;
> > 
> > val is a uint64_t so we don''t support more than two submaps,
which is ok
> > for now but the elf note needs to include a way to grow beyond that in
a
> > forward and backward compatible way (lest we grow a third interface
for
> > this in the future!).
> 
> Hmm, indeed - we''d have to improve elf_note_numeric() to be
> forward compatible.
> 
> > Notes have a length field and we support 1,2 and 4 byte numerical
notes
> > but here I think we need to add support for arbitrary length arrays on
> > numerical values.
> 
> Easiest would seem to be to have the caller (optionally) specify a unit
> size and index. What do you think?
something like
	for (idx = 0; idx<note.len/sizeof(uint32_t); idx++)
		val = elf_note_numeric(sizeof(uint32_t), idx);
                ...use val...
???

I was thinking of making the "int str" in note_desc be an enum with
"u32_array" (or whatever) as a new value (allowing for others in the
future) but what you suggest works too.
> >>  /* 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
> > 
> > This still strikes me as odd because unprivileged is a subset of
> > privileged (I understand the backwards compatibility argument for
having
> > it this way though). Really XENFEAT_unprivileged is the
> > "XENFEAT_privileged feature bit is supported" meta-feature
flag.
> 
> No, I don''t view it that way - in the Linux ports, the meaning of
> the respective config options is such that privileged includes
> unprivileged, but that''s a guest OS decision, not one the
interface
> should dictate.
Hmm, I suppose. I''m not sure it would be possible to implement a guest
which was able to run as a dom0 but didn''t by necessity also implement
enough stuff to work as a domU (at least not far enough to print
something useful).

Which reminds me -- when you boot a non-dom0 (but domU) capable kernel
as dom0 does it not get far enough to print something to its console?
IOW could we avoid this whole mess by "fixing" it in the guest?

Ian.
> > Do we really mean "privileged" here, or do we mean
"dom0", I know the
> > two are tied together today but will this be the case as we
disaggregate
> > more and more? Does this flag really mean "can drive APICs and
run ACPI
> > code etc"? Which is distinct from the ability to drive hardware
> > generally etc.
> 
> No, this is really only meaningful as a Dom0-capability indication in
> today''s sense. Splitting this will probably yield the whole
privileged/
> unprivileged distinction bogus, and hence kernels supporting this
> would probably be required to just always set both flags.
> 
> If the feature naming is a problem, we could certainly rename them
> into "dom0" and "domU", to distinguish the two ways of
getting
> loaded.
> 
> Jan


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

>>> On 19.07.11 at 12:44, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> On Tue, 2011-07-19 at 11:24 +0100, Jan Beulich wrote:
>> >>> On 19.07.11 at 11:28, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
>> > On Tue, 2011-07-19 at 10:00 +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.
>> >> 
>> >> v2: Due to the way elf_xen_parse_features() worked up to now
(getting
>> >> fixed here), adding features indications to the old, string
based ELF
>> >> note would make the respective kernel unusable on older
hypervisors.
>> > 
>> > What was the failure mode? Can we not fix it (with suitable
backport
>> > recommendations) rather than adding a new duplicated interface?
>> 
>> Adding a supported feature Xen doesn''t understand leads to a
>> "cannot load Dom0 kernel" without any indication what was
actually
>> wrong with the kernel.
>> 
>> The fix is trivial (this patch''s change to
elf_xen_parse_features()),
>> but expecting everyone to backport this to (perhaps very) old
>> hypervisors didn''t seem realistic to me.
> 
> Backporting a trivial fix like this is pretty easy, especially to this
> code which has been reasonably static for a long time.
> 
> People who need this fix are either building a modern kernel themselves
> (in which case I expect they can cope with getting a fixed hypervisor
> too) or they are getting it from their distro and then the packages
> dependencies will cause the necessary hypervisor fix to get pulled in.
Not really. Just consider virtualization hosters who run on a very old
hypervisor, but offer people to use modern kernels. You just can''t
imply that a modern kernel will only run on half-way modern
hypervisors, or that if you have control over the (DomU) kernel, you
also have control over the hypervisor.

>> >>  /* 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
>> > 
>> > This still strikes me as odd because unprivileged is a subset of
>> > privileged (I understand the backwards compatibility argument for
having
>> > it this way though). Really XENFEAT_unprivileged is the
>> > "XENFEAT_privileged feature bit is supported"
meta-feature flag.
>> 
>> No, I don''t view it that way - in the Linux ports, the meaning
of
>> the respective config options is such that privileged includes
>> unprivileged, but that''s a guest OS decision, not one the
interface
>> should dictate.
> 
> Hmm, I suppose. I''m not sure it would be possible to implement a
guest
> which was able to run as a dom0 but didn''t by necessity also
implement
> enough stuff to work as a domU (at least not far enough to print
> something useful).
> 
> Which reminds me -- when you boot a non-dom0 (but domU) capable kernel
> as dom0 does it not get far enough to print something to its console?
> IOW could we avoid this whole mess by "fixing" it in the guest?
Without "earlyprintk=xen" it likely won''t get far enough. And
earlyprintk=xen really makes no sense on a DomU-only kernel, so
could even be unavailable. Further the guest likely wouldn''t know of
a VGA or VESA fb (again because of being DomU-only), and hence it
may not have a usable console interface at all when booted as Dom0.

Jan


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

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.

v2: Due to the way elf_xen_parse_features() worked up to now (getting
fixed here), adding features indications to the old, string based ELF
note would make the respective kernel unusable on older hypervisors.
For that reason, a new ELF Note is being introduced that allows
specifying supported features as a bit array instead (with features
unknown to the hypervisor simply ignored, as now also done by
elf_xen_parse_features(), whereas here unknown kernel-required
features still keep the kernel [and hence VM] from booting).

v3: Introduce and use elf_note_numeric_array() to be forward
compatible (or else an old hypervisor wouldn''t be able to parse kernel
specified features occupying more than 64 bits - thanks, 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]);
@@ -83,7 +85,7 @@ int elf_xen_parse_features(const char *f
                 }
             }
         }
-        if ( i == elf_xen_features )
+        if ( i == elf_xen_features && required && feature[0] ==
''!'' )
             return -1;
     }
 
@@ -114,6 +116,7 @@ int elf_xen_parse_note(struct elf_binary
         [XEN_ELFNOTE_LOADER] = { "LOADER", 1},
         [XEN_ELFNOTE_PAE_MODE] = { "PAE_MODE", 1},
         [XEN_ELFNOTE_FEATURES] = { "FEATURES", 1},
+        [XEN_ELFNOTE_SUPPORTED_FEATURES] = { "SUPPORTED_FEATURES",
0},
         [XEN_ELFNOTE_BSD_SYMTAB] = { "BSD_SYMTAB", 1},
         [XEN_ELFNOTE_SUSPEND_CANCEL] = { "SUSPEND_CANCEL", 0 },
         [XEN_ELFNOTE_MOD_START_PFN] = { "MOD_START_PFN", 0 },
@@ -122,6 +125,7 @@ int elf_xen_parse_note(struct elf_binary
 
     const char *str = NULL;
     uint64_t val = 0;
+    unsigned int i;
     int type = elf_uval(elf, note, type);
 
     if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
@@ -200,6 +204,12 @@ int elf_xen_parse_note(struct elf_binary
             return -1;
         break;
 
+    case XEN_ELFNOTE_SUPPORTED_FEATURES:
+        for ( i = 0; i < XENFEAT_NR_SUBMAPS; ++i )
+            parms->f_supported[i] |= elf_note_numeric_array(
+                elf, note, sizeof(*parms->f_supported), i);
+        break;
+
     }
     return 0;
 }
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -227,6 +227,27 @@ uint64_t elf_note_numeric(struct elf_bin
         return 0;
     }
 }
+
+uint64_t elf_note_numeric_array(struct elf_binary *elf, const elf_note *note,
+                                unsigned int unitsz, unsigned int idx)
+{
+    const void *desc = elf_note_desc(elf, note);
+    int descsz = elf_uval(elf, note, descsz);
+
+    if ( descsz % unitsz || idx >= descsz / unitsz )
+        return 0;
+    switch (unitsz)
+    {
+    case 1:
+    case 2:
+    case 4:
+    case 8:
+        return elf_access_unsigned(elf, desc, idx * unitsz, unitsz);
+    default:
+        return 0;
+    }
+}
+
 const elf_note *elf_note_next(struct elf_binary *elf, const elf_note * note)
 {
     int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
--- a/xen/include/public/elfnote.h
+++ b/xen/include/public/elfnote.h
@@ -179,9 +179,22 @@
 #define XEN_ELFNOTE_MOD_START_PFN 16
 
 /*
+ * The features supported by this kernel (numeric).
+ *
+ * Other than XEN_ELFNOTE_FEATURES on pre-4.2 Xen, this note allows a
+ * kernel to specify support for features that older hypervisors don''t
+ * know about. The set of features 4.2 and newer hypervisors will
+ * consider supported by the kernel is the combination of the sets
+ * specified through this and the string note.
+ *
+ * LEGACY: FEATURES
+ */
+#define XEN_ELFNOTE_SUPPORTED_FEATURES 17
+
+/*
  * The number of the highest elfnote defined.
  */
-#define XEN_ELFNOTE_MAX XEN_ELFNOTE_MOD_START_PFN
+#define XEN_ELFNOTE_MAX XEN_ELFNOTE_SUPPORTED_FEATURES
 
 /*
  * System information exported through crash notes.
--- 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
 
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -179,6 +179,8 @@ const elf_sym *elf_sym_by_index(struct e
 const char *elf_note_name(struct elf_binary *elf, const elf_note * note);
 const void *elf_note_desc(struct elf_binary *elf, const elf_note * note);
 uint64_t elf_note_numeric(struct elf_binary *elf, const elf_note * note);
+uint64_t elf_note_numeric_array(struct elf_binary *, const elf_note *,
+                                unsigned int unitsz, unsigned int idx);
 const elf_note *elf_note_next(struct elf_binary *elf, const elf_note * note);
 
 int elf_is_elfbinary(const void *image);




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

Oops, $subject should have said [PATCH, v3] ...
>>> On 19.07.11 at 15:16, "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.
> 
> v2: Due to the way elf_xen_parse_features() worked up to now (getting
> fixed here), adding features indications to the old, string based ELF
> note would make the respective kernel unusable on older hypervisors.
> For that reason, a new ELF Note is being introduced that allows
> specifying supported features as a bit array instead (with features
> unknown to the hypervisor simply ignored, as now also done by
> elf_xen_parse_features(), whereas here unknown kernel-required
> features still keep the kernel [and hence VM] from booting).
> 
> v3: Introduce and use elf_note_numeric_array() to be forward
> compatible (or else an old hypervisor wouldn''t be able to parse
kernel
> specified features occupying more than 64 bits - thanks, 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]);
> @@ -83,7 +85,7 @@ int elf_xen_parse_features(const char *f
>                  }
>              }
>          }
> -        if ( i == elf_xen_features )
> +        if ( i == elf_xen_features && required &&
feature[0] == ''!'' )
>              return -1;
>      }
>  
> @@ -114,6 +116,7 @@ int elf_xen_parse_note(struct elf_binary
>          [XEN_ELFNOTE_LOADER] = { "LOADER", 1},
>          [XEN_ELFNOTE_PAE_MODE] = { "PAE_MODE", 1},
>          [XEN_ELFNOTE_FEATURES] = { "FEATURES", 1},
> +        [XEN_ELFNOTE_SUPPORTED_FEATURES] = {
"SUPPORTED_FEATURES", 0},
>          [XEN_ELFNOTE_BSD_SYMTAB] = { "BSD_SYMTAB", 1},
>          [XEN_ELFNOTE_SUSPEND_CANCEL] = { "SUSPEND_CANCEL", 0 },
>          [XEN_ELFNOTE_MOD_START_PFN] = { "MOD_START_PFN", 0 },
> @@ -122,6 +125,7 @@ int elf_xen_parse_note(struct elf_binary
>  
>      const char *str = NULL;
>      uint64_t val = 0;
> +    unsigned int i;
>      int type = elf_uval(elf, note, type);
>  
>      if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
> @@ -200,6 +204,12 @@ int elf_xen_parse_note(struct elf_binary
>              return -1;
>          break;
>  
> +    case XEN_ELFNOTE_SUPPORTED_FEATURES:
> +        for ( i = 0; i < XENFEAT_NR_SUBMAPS; ++i )
> +            parms->f_supported[i] |= elf_note_numeric_array(
> +                elf, note, sizeof(*parms->f_supported), i);
> +        break;
> +
>      }
>      return 0;
>  }
> --- a/xen/common/libelf/libelf-tools.c
> +++ b/xen/common/libelf/libelf-tools.c
> @@ -227,6 +227,27 @@ uint64_t elf_note_numeric(struct elf_bin
>          return 0;
>      }
>  }
> +
> +uint64_t elf_note_numeric_array(struct elf_binary *elf, const elf_note 
> *note,
> +                                unsigned int unitsz, unsigned int idx)
> +{
> +    const void *desc = elf_note_desc(elf, note);
> +    int descsz = elf_uval(elf, note, descsz);
> +
> +    if ( descsz % unitsz || idx >= descsz / unitsz )
> +        return 0;
> +    switch (unitsz)
> +    {
> +    case 1:
> +    case 2:
> +    case 4:
> +    case 8:
> +        return elf_access_unsigned(elf, desc, idx * unitsz, unitsz);
> +    default:
> +        return 0;
> +    }
> +}
> +
>  const elf_note *elf_note_next(struct elf_binary *elf, const elf_note * 
> note)
>  {
>      int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
> --- a/xen/include/public/elfnote.h
> +++ b/xen/include/public/elfnote.h
> @@ -179,9 +179,22 @@
>  #define XEN_ELFNOTE_MOD_START_PFN 16
>  
>  /*
> + * The features supported by this kernel (numeric).
> + *
> + * Other than XEN_ELFNOTE_FEATURES on pre-4.2 Xen, this note allows a
> + * kernel to specify support for features that older hypervisors
don''t
> + * know about. The set of features 4.2 and newer hypervisors will
> + * consider supported by the kernel is the combination of the sets
> + * specified through this and the string note.
> + *
> + * LEGACY: FEATURES
> + */
> +#define XEN_ELFNOTE_SUPPORTED_FEATURES 17
> +
> +/*
>   * The number of the highest elfnote defined.
>   */
> -#define XEN_ELFNOTE_MAX XEN_ELFNOTE_MOD_START_PFN
> +#define XEN_ELFNOTE_MAX XEN_ELFNOTE_SUPPORTED_FEATURES
>  
>  /*
>   * System information exported through crash notes.
> --- 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
>  
> --- a/xen/include/xen/libelf.h
> +++ b/xen/include/xen/libelf.h
> @@ -179,6 +179,8 @@ const elf_sym *elf_sym_by_index(struct e
>  const char *elf_note_name(struct elf_binary *elf, const elf_note * note);
>  const void *elf_note_desc(struct elf_binary *elf, const elf_note * note);
>  uint64_t elf_note_numeric(struct elf_binary *elf, const elf_note * note);
> +uint64_t elf_note_numeric_array(struct elf_binary *, const elf_note *,
> +                                unsigned int unitsz, unsigned int idx);
>  const elf_note *elf_note_next(struct elf_binary *elf, const elf_note * 
> note);
>  
>  int elf_is_elfbinary(const void *image);


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

On Tue, 2011-07-19 at 14:17 +0100, Jan Beulich wrote:
> Oops, $subject should have said [PATCH, v3] ...
> 
> >>> On 19.07.11 at 15:16, "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.
Actually, since you are introducing a new interface to the feature bits
_and_ it is not possible to add these new bits to the old interface
anyway can''t we just have XENFEAT_privileged and require that guest
kernels using the new interface ensure that bit correctly represents the
configuration? IOW backwards compatilibilty is ensure through the
presence/absence of XEN_ELFNOTE_SUPPORTED_FEATURES.

Ian.
> > 
> > v2: Due to the way elf_xen_parse_features() worked up to now (getting
> > fixed here), adding features indications to the old, string based ELF
> > note would make the respective kernel unusable on older hypervisors.
> > For that reason, a new ELF Note is being introduced that allows
> > specifying supported features as a bit array instead (with features
> > unknown to the hypervisor simply ignored, as now also done by
> > elf_xen_parse_features(), whereas here unknown kernel-required
> > features still keep the kernel [and hence VM] from booting).
> > 
> > v3: Introduce and use elf_note_numeric_array() to be forward
> > compatible (or else an old hypervisor wouldn''t be able to
parse kernel
> > specified features occupying more than 64 bits - thanks, 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]);
> > @@ -83,7 +85,7 @@ int elf_xen_parse_features(const char *f
> >                  }
> >              }
> >          }
> > -        if ( i == elf_xen_features )
> > +        if ( i == elf_xen_features && required &&
feature[0] == ''!'' )
> >              return -1;
> >      }
> >  
> > @@ -114,6 +116,7 @@ int elf_xen_parse_note(struct elf_binary
> >          [XEN_ELFNOTE_LOADER] = { "LOADER", 1},
> >          [XEN_ELFNOTE_PAE_MODE] = { "PAE_MODE", 1},
> >          [XEN_ELFNOTE_FEATURES] = { "FEATURES", 1},
> > +        [XEN_ELFNOTE_SUPPORTED_FEATURES] = {
"SUPPORTED_FEATURES", 0},
> >          [XEN_ELFNOTE_BSD_SYMTAB] = { "BSD_SYMTAB", 1},
> >          [XEN_ELFNOTE_SUSPEND_CANCEL] = { "SUSPEND_CANCEL",
0 },
> >          [XEN_ELFNOTE_MOD_START_PFN] = { "MOD_START_PFN", 0
},
> > @@ -122,6 +125,7 @@ int elf_xen_parse_note(struct elf_binary
> >  
> >      const char *str = NULL;
> >      uint64_t val = 0;
> > +    unsigned int i;
> >      int type = elf_uval(elf, note, type);
> >  
> >      if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
> > @@ -200,6 +204,12 @@ int elf_xen_parse_note(struct elf_binary
> >              return -1;
> >          break;
> >  
> > +    case XEN_ELFNOTE_SUPPORTED_FEATURES:
> > +        for ( i = 0; i < XENFEAT_NR_SUBMAPS; ++i )
> > +            parms->f_supported[i] |= elf_note_numeric_array(
> > +                elf, note, sizeof(*parms->f_supported), i);
> > +        break;
> > +
> >      }
> >      return 0;
> >  }
> > --- a/xen/common/libelf/libelf-tools.c
> > +++ b/xen/common/libelf/libelf-tools.c
> > @@ -227,6 +227,27 @@ uint64_t elf_note_numeric(struct elf_bin
> >          return 0;
> >      }
> >  }
> > +
> > +uint64_t elf_note_numeric_array(struct elf_binary *elf, const
elf_note
> > *note,
> > +                                unsigned int unitsz, unsigned int
idx)
> > +{
> > +    const void *desc = elf_note_desc(elf, note);
> > +    int descsz = elf_uval(elf, note, descsz);
> > +
> > +    if ( descsz % unitsz || idx >= descsz / unitsz )
> > +        return 0;
> > +    switch (unitsz)
> > +    {
> > +    case 1:
> > +    case 2:
> > +    case 4:
> > +    case 8:
> > +        return elf_access_unsigned(elf, desc, idx * unitsz, unitsz);
> > +    default:
> > +        return 0;
> > +    }
> > +}
> > +
> >  const elf_note *elf_note_next(struct elf_binary *elf, const elf_note
*
> > note)
> >  {
> >      int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
> > --- a/xen/include/public/elfnote.h
> > +++ b/xen/include/public/elfnote.h
> > @@ -179,9 +179,22 @@
> >  #define XEN_ELFNOTE_MOD_START_PFN 16
> >  
> >  /*
> > + * The features supported by this kernel (numeric).
> > + *
> > + * Other than XEN_ELFNOTE_FEATURES on pre-4.2 Xen, this note allows a
> > + * kernel to specify support for features that older hypervisors
don''t
> > + * know about. The set of features 4.2 and newer hypervisors will
> > + * consider supported by the kernel is the combination of the sets
> > + * specified through this and the string note.
> > + *
> > + * LEGACY: FEATURES
> > + */
> > +#define XEN_ELFNOTE_SUPPORTED_FEATURES 17
> > +
> > +/*
> >   * The number of the highest elfnote defined.
> >   */
> > -#define XEN_ELFNOTE_MAX XEN_ELFNOTE_MOD_START_PFN
> > +#define XEN_ELFNOTE_MAX XEN_ELFNOTE_SUPPORTED_FEATURES
> >  
> >  /*
> >   * System information exported through crash notes.
> > --- 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
> >  
> > --- a/xen/include/xen/libelf.h
> > +++ b/xen/include/xen/libelf.h
> > @@ -179,6 +179,8 @@ const elf_sym *elf_sym_by_index(struct e
> >  const char *elf_note_name(struct elf_binary *elf, const elf_note *
note);
> >  const void *elf_note_desc(struct elf_binary *elf, const elf_note *
note);
> >  uint64_t elf_note_numeric(struct elf_binary *elf, const elf_note *
note);
> > +uint64_t elf_note_numeric_array(struct elf_binary *, const elf_note
*,
> > +                                unsigned int unitsz, unsigned int
idx);
> >  const elf_note *elf_note_next(struct elf_binary *elf, const elf_note
*
> > note);
> >  
> >  int elf_is_elfbinary(const void *image);
> 
> 


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

>>> On 20.07.11 at 12:39, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> On Tue, 2011-07-19 at 14:17 +0100, Jan Beulich wrote:
>> Oops, $subject should have said [PATCH, v3] ...
>> 
>> >>> On 19.07.11 at 15:16, "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.
> 
> Actually, since you are introducing a new interface to the feature bits
> _and_ it is not possible to add these new bits to the old interface
> anyway can''t we just have XENFEAT_privileged and require that
guest
> kernels using the new interface ensure that bit correctly represents the
> configuration? IOW backwards compatilibilty is ensure through the
> presence/absence of XEN_ELFNOTE_SUPPORTED_FEATURES.
At the risk of repeating myself - the notion of "privileged" implying
ability to run unprivileged is a Linux one, and I don''t want to embed
such an implication in the interface.

Jan


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

On Wed, 2011-07-20 at 13:50 +0100, Jan Beulich wrote: 
> >>> On 20.07.11 at 12:39, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> > On Tue, 2011-07-19 at 14:17 +0100, Jan Beulich wrote:
> >> Oops, $subject should have said [PATCH, v3] ...
> >> 
> >> >>> On 19.07.11 at 15:16, "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.
> > 
> > Actually, since you are introducing a new interface to the feature
bits
> > _and_ it is not possible to add these new bits to the old interface
> > anyway can''t we just have XENFEAT_privileged and require that
guest
> > kernels using the new interface ensure that bit correctly represents
the
> > configuration? IOW backwards compatilibilty is ensure through the
> > presence/absence of XEN_ELFNOTE_SUPPORTED_FEATURES.
> 
> At the risk of repeating myself - the notion of "privileged"
implying
> ability to run unprivileged is a Linux one, and I don''t want to
embed
> such an implication in the interface.
At the risk of repeating myself -- which functionality does domU require
which is not also already necessary for a dom0 -- at least to the point
of failing gracefully during boot?

You also have not explained _why_ a dom0-only guest would be a useful
thing to have and to add extra complexity to our interfaces for, it''s
obviously very much a corner case.

If we choose to do so we can decree that a dom0-only capable guest must
also have sufficient domU functionality to print a message when running
as domU and if they choose not to do this then it only impacts that
guest and its users (which IMHO provides sufficient pressure on guest
implementers to do the right thing).

You say it is a Linux notion that dom0 implies domU but I am not aware
of any PV OS which supports dom0 that doesn''t also support domU, do you
have specific examples of OSes which are dom0-only?

Ian.




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

>>> On 21.07.11 at 10:01, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> On Wed, 2011-07-20 at 13:50 +0100, Jan Beulich wrote: 
>> >>> On 20.07.11 at 12:39, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
>> > On Tue, 2011-07-19 at 14:17 +0100, Jan Beulich wrote:
>> >> Oops, $subject should have said [PATCH, v3] ...
>> >> 
>> >> >>> On 19.07.11 at 15:16, "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.
>> > 
>> > Actually, since you are introducing a new interface to the feature
bits
>> > _and_ it is not possible to add these new bits to the old
interface
>> > anyway can''t we just have XENFEAT_privileged and require
that guest
>> > kernels using the new interface ensure that bit correctly
represents the
>> > configuration? IOW backwards compatilibilty is ensure through the
>> > presence/absence of XEN_ELFNOTE_SUPPORTED_FEATURES.
>> 
>> At the risk of repeating myself - the notion of "privileged"
implying
>> ability to run unprivileged is a Linux one, and I don''t want
to embed
>> such an implication in the interface.
> 
> At the risk of repeating myself -- which functionality does domU require
> which is not also already necessary for a dom0 -- at least to the point
> of failing gracefully during boot?
Graceful failure requires access to some user visible output device. A
Dom0-only kernel may e.g. not have any frontend drivers (particularly
no xenfb one, and the guest and/or kernel config may not allow other
virtualized output mechanisms).
> You also have not explained _why_ a dom0-only guest would be a useful
> thing to have and to add extra complexity to our interfaces for,
it''s
> obviously very much a corner case.
It''s really a decision between having efficient code (i.e. as little
unused code as possible in a kernel suiting a particular need) and
having a (relatively) general-purpose kernel.
> If we choose to do so we can decree that a dom0-only capable guest must
> also have sufficient domU functionality to print a message when running
> as domU and if they choose not to do this then it only impacts that
> guest and its users (which IMHO provides sufficient pressure on guest
> implementers to do the right thing).
> 
> You say it is a Linux notion that dom0 implies domU but I am not aware
> of any PV OS which supports dom0 that doesn''t also support domU,
do you
> have specific examples of OSes which are dom0-only?
No, I''m not aware of any existing ones, but I also wasn''t in
favor of
the move to imply unprivileged capabilities when Linux is configured
as privileged guest (iirc this wasn''t the case from the very
beginning).

And again, imo an interface like the hypervisor''s shouldn''t
dictate any
kind of policy on the guest OSes.

Jan


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

On Thu, 2011-07-21 at 09:16 +0100, Jan Beulich wrote:
> >>> On 21.07.11 at 10:01, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> > On Wed, 2011-07-20 at 13:50 +0100, Jan Beulich wrote: 
> >> >>> On 20.07.11 at 12:39, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> >> > On Tue, 2011-07-19 at 14:17 +0100, Jan Beulich wrote:
> >> >> Oops, $subject should have said [PATCH, v3] ...
> >> >> 
> >> >> >>> On 19.07.11 at 15:16, "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.
> >> > 
> >> > Actually, since you are introducing a new interface to the
feature bits
> >> > _and_ it is not possible to add these new bits to the old
interface
> >> > anyway can''t we just have XENFEAT_privileged and
require that guest
> >> > kernels using the new interface ensure that bit correctly
represents the
> >> > configuration? IOW backwards compatilibilty is ensure through
the
> >> > presence/absence of XEN_ELFNOTE_SUPPORTED_FEATURES.
> >> 
> >> At the risk of repeating myself - the notion of
"privileged" implying
> >> ability to run unprivileged is a Linux one, and I don''t
want to embed
> >> such an implication in the interface.
> > 
> > At the risk of repeating myself -- which functionality does domU
require
> > which is not also already necessary for a dom0 -- at least to the
point
> > of failing gracefully during boot?
> 
> Graceful failure requires access to some user visible output device. A
> Dom0-only kernel may e.g. not have any frontend drivers (particularly
> no xenfb one, and the guest and/or kernel config may not allow other
> virtualized output mechanisms).
Your Linux side patch sets the unprivileged guest feature independently
of CONFIG_XEN_FRAMEBUFFER and/or CONFIG_HVC_XEN so the proposed
interface is already providing opportunities for confusion/ambiguity.
> > You also have not explained _why_ a dom0-only guest would be a useful
> > thing to have and to add extra complexity to our interfaces for,
it''s
> > obviously very much a corner case.
> 
> It''s really a decision between having efficient code (i.e. as
little
> unused code as possible in a kernel suiting a particular need) and
> having a (relatively) general-purpose kernel.
We are talking about half a dozen lines of code to spit out a static
string ("I don''t support domU operation") to the domU and/or
a guest
side decision to omit sch code on the understanding that the
disadvantage will be less error reporting when a minimal kernel suiting
a particular need is used in some other context.
> > If we choose to do so we can decree that a dom0-only capable guest
must
> > also have sufficient domU functionality to print a message when
running
> > as domU and if they choose not to do this then it only impacts that
> > guest and its users (which IMHO provides sufficient pressure on guest
> > implementers to do the right thing).
> > 
> > You say it is a Linux notion that dom0 implies domU but I am not aware
> > of any PV OS which supports dom0 that doesn''t also support
domU, do you
> > have specific examples of OSes which are dom0-only?
> 
> No, I''m not aware of any existing ones, but I also wasn''t
in favor of
> the move to imply unprivileged capabilities when Linux is configured
> as privileged guest (iirc this wasn''t the case from the very
beginning).
> 
> And again, imo an interface like the hypervisor''s
shouldn''t dictate any
> kind of policy on the guest OSes.
The only policy here is "if you want to be user friendly you must
minimally arrange to output a simple domU console message indicating
lack of domU support if you support dom0 operation only".

That''s not so much a policy as pushing responsibility down into the
guest kernel in an extremely unlikely configuration.

Ian.


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

On 21/07/2011 09:16, "Jan Beulich" <JBeulich@novell.com> wrote:
>> You say it is a Linux notion that dom0 implies domU but I am not aware
>> of any PV OS which supports dom0 that doesn''t also support
domU, do you
>> have specific examples of OSes which are dom0-only?
> 
> No, I''m not aware of any existing ones, but I also wasn''t
in favor of
> the move to imply unprivileged capabilities when Linux is configured
> as privileged guest (iirc this wasn''t the case from the very
beginning).
> 
> And again, imo an interface like the hypervisor''s
shouldn''t dictate any
> kind of policy on the guest OSes.
My own issue with the unprivileged flag is that I''m not clear what it
actually means. When would you *not* set it? I mean it looks in the Linux
side you set it unconditionally right now. What''s the point? Why not
remove
the flag and introduce it when we have good reason and can attach meaningful
semantics to it?

There we are, we''re two against one now ;-)

 -- Keir



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

>>> On 21.07.11 at 10:38, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> On Thu, 2011-07-21 at 09:16 +0100, Jan Beulich wrote:
>> > You also have not explained _why_ a dom0-only guest would be a
useful
>> > thing to have and to add extra complexity to our interfaces for,
it''s
>> > obviously very much a corner case.
>> 
>> It''s really a decision between having efficient code (i.e. as
little
>> unused code as possible in a kernel suiting a particular need) and
>> having a (relatively) general-purpose kernel.
> 
> We are talking about half a dozen lines of code to spit out a static
> string ("I don''t support domU operation") to the domU
and/or a guest
You still didn''t tell where such a message would show up: Printing
such a message isn''t a big deal, but pointless if it goes into
no-where.
If instead the hypervisor (for Dom0) or the tools (for DomU-s) print
something, this will be visible in a known place.

Jan



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

On 21/07/2011 09:50, "Keir Fraser" <keir@xen.org> wrote:
> On 21/07/2011 09:16, "Jan Beulich" <JBeulich@novell.com>
wrote:
> 
>>> You say it is a Linux notion that dom0 implies domU but I am not
aware
>>> of any PV OS which supports dom0 that doesn''t also support
domU, do you
>>> have specific examples of OSes which are dom0-only?
>> 
>> No, I''m not aware of any existing ones, but I also
wasn''t in favor of
>> the move to imply unprivileged capabilities when Linux is configured
>> as privileged guest (iirc this wasn''t the case from the very
beginning).
>> 
>> And again, imo an interface like the hypervisor''s
shouldn''t dictate any
>> kind of policy on the guest OSes.
> 
> My own issue with the unprivileged flag is that I''m not clear what
it
> actually means. When would you *not* set it? I mean it looks in the Linux
> side you set it unconditionally right now. What''s the point? Why
not remove
> the flag and introduce it when we have good reason and can attach
meaningful
> semantics to it?
A further killing blow: the hypervisor patch defined unprivileged as !dom0.
Well, there are many different capabilities and devices that a domU may be
granted. You might be passing through a VGA adaptor and SRIOV NIC and run
out of ramdisk for example, in which case the domU might quite validly have
no PV frontend devices.

Another thing, given that privileged is quite a broad term, I wonder whether
the ''privileged'' feature should be called something else? Like
''dom0_interface''? It would be a more precise definition maybe?
Passing
through devices to a domU could be termed a privilege after all, for
example.

 -- Keir
> There we are, we''re two against one now ;-)
> 
>  -- Keir
> 
> 


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

>>> On 21.07.11 at 10:50, Keir Fraser <keir@xen.org> wrote:
> On 21/07/2011 09:16, "Jan Beulich" <JBeulich@novell.com>
wrote:
> 
>>> You say it is a Linux notion that dom0 implies domU but I am not
aware
>>> of any PV OS which supports dom0 that doesn''t also support
domU, do you
>>> have specific examples of OSes which are dom0-only?
>> 
>> No, I''m not aware of any existing ones, but I also
wasn''t in favor of
>> the move to imply unprivileged capabilities when Linux is configured
>> as privileged guest (iirc this wasn''t the case from the very
beginning).
>> 
>> And again, imo an interface like the hypervisor''s
shouldn''t dictate any
>> kind of policy on the guest OSes.
> 
> My own issue with the unprivileged flag is that I''m not clear what
it
> actually means. When would you *not* set it? I mean it looks in the Linux
> side you set it unconditionally right now. What''s the point? Why
not remove
> the flag and introduce it when we have good reason and can attach
meaningful
> semantics to it?
Again - you''re talking about an actual guest side implementation
(which,
in this particular case, has to honor how the rest of the implementation
is done, i.e. it has to set the flag unconditionally). I''m talking
about an
abstract interface definition that should suit everyone (existing as well
as yet to come).
> There we are, we''re two against one now ;-)
Still hoping you both get my point. If not, I''ll have to give in
without
being convinced.

Jan


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

On Thu, 2011-07-21 at 09:55 +0100, Jan Beulich wrote:
> >>> On 21.07.11 at 10:38, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> > On Thu, 2011-07-21 at 09:16 +0100, Jan Beulich wrote:
> >> > You also have not explained _why_ a dom0-only guest would be
a useful
> >> > thing to have and to add extra complexity to our interfaces
for, it''s
> >> > obviously very much a corner case.
> >> 
> >> It''s really a decision between having efficient code
(i.e. as little
> >> unused code as possible in a kernel suiting a particular need) and
> >> having a (relatively) general-purpose kernel.
> > 
> > We are talking about half a dozen lines of code to spit out a static
> > string ("I don''t support domU operation") to the
domU and/or a guest
> 
> You still didn''t tell where such a message would show up: Printing
> such a message isn''t a big deal, but pointless if it goes into
no-where.
> If instead the hypervisor (for Dom0) or the tools (for DomU-s) print
> something, this will be visible in a known place.
If someone runs a dom0-only kernel as a domU (presumably by mistake)
then they are naturally going to look in the domU console ring for error
messages when it doesn''t work (because they thought they were running a
domU, where else would they look?). Placing that message there is
basically a memcpy and an evtchn_notify.

If someone runs a domU only kernel as a dom0 then the XENFEAT_privileged
(or whatever it gets called) is precisely enough to allow the hypervisor
to say something useful in that case.

Ian.



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

>>> On 21.07.11 at 10:55, Keir Fraser <keir@xen.org> wrote:
> On 21/07/2011 09:50, "Keir Fraser" <keir@xen.org> wrote:
> 
>> On 21/07/2011 09:16, "Jan Beulich"
<JBeulich@novell.com> wrote:
>> 
>>>> You say it is a Linux notion that dom0 implies domU but I am
not aware
>>>> of any PV OS which supports dom0 that doesn''t also
support domU, do you
>>>> have specific examples of OSes which are dom0-only?
>>> 
>>> No, I''m not aware of any existing ones, but I also
wasn''t in favor of
>>> the move to imply unprivileged capabilities when Linux is
configured
>>> as privileged guest (iirc this wasn''t the case from the
very beginning).
>>> 
>>> And again, imo an interface like the hypervisor''s
shouldn''t dictate any
>>> kind of policy on the guest OSes.
>> 
>> My own issue with the unprivileged flag is that I''m not clear
what it
>> actually means. When would you *not* set it? I mean it looks in the
Linux
>> side you set it unconditionally right now. What''s the point?
Why not remove
>> the flag and introduce it when we have good reason and can attach
meaningful
>> semantics to it?
> 
> A further killing blow: the hypervisor patch defined unprivileged as !dom0.
> Well, there are many different capabilities and devices that a domU may be
> granted. You might be passing through a VGA adaptor and SRIOV NIC and run
> out of ramdisk for example, in which case the domU might quite validly have
> no PV frontend devices.
> 
> Another thing, given that privileged is quite a broad term, I wonder
whether
> the ''privileged'' feature should be called something else?
Like
> ''dom0_interface''? It would be a more precise definition
maybe? Passing
> through devices to a domU could be termed a privilege after all, for
> example.
I agree that if we''re going to go with just a single flag, then
renaming it
the way you suggest certainly makes sense.

Jan


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

On 21/07/2011 10:01, "Jan Beulich" <JBeulich@novell.com> wrote:
>> My own issue with the unprivileged flag is that I''m not clear
what it
>> actually means. When would you *not* set it? I mean it looks in the
Linux
>> side you set it unconditionally right now. What''s the point?
Why not remove
>> the flag and introduce it when we have good reason and can attach
meaningful
>> semantics to it?
> 
> Again - you''re talking about an actual guest side implementation
(which,
> in this particular case, has to honor how the rest of the implementation
> is done, i.e. it has to set the flag unconditionally). I''m talking
about an
> abstract interface definition that should suit everyone (existing as well
> as yet to come).
I''m afraid my view is that !dom0 operation is a mode that every PV
guest is
capable of -- that''s the definition of what a PV guest is, to my mind!

Like I said in my other email, pinning it down more precisely -- e.g., must
have PV drivers -- will have counterexamples.

I don''t know though --- maybe a distro could have dom0 and domU
separate
kernel builds, and want to set the flags differently for each. I wonder how
much of a long shot that use case is.

 -- Keir



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

On 21/07/2011 10:10, "Keir Fraser" <keir@xen.org> wrote:
> On 21/07/2011 10:01, "Jan Beulich" <JBeulich@novell.com>
wrote:
> 
>>> My own issue with the unprivileged flag is that I''m not
clear what it
>>> actually means. When would you *not* set it? I mean it looks in the
Linux
>>> side you set it unconditionally right now. What''s the
point? Why not remove
>>> the flag and introduce it when we have good reason and can attach
meaningful
>>> semantics to it?
>> 
>> Again - you''re talking about an actual guest side
implementation (which,
>> in this particular case, has to honor how the rest of the
implementation
>> is done, i.e. it has to set the flag unconditionally). I''m
talking about an
>> abstract interface definition that should suit everyone (existing as
well
>> as yet to come).
> 
> I''m afraid my view is that !dom0 operation is a mode that every PV
guest is
> capable of -- that''s the definition of what a PV guest is, to my
mind!
> 
> Like I said in my other email, pinning it down more precisely -- e.g., must
> have PV drivers -- will have counterexamples.
> 
> I don''t know though --- maybe a distro could have dom0 and domU
separate
> kernel builds, and want to set the flags differently for each. I wonder how
> much of a long shot that use case is.
How about having a XEN_ELFNOTE_REQUIRED_FEATURES as well as
supported_features? Then you can define just one flag,
XENFEAT_dom0_interface or somesuch, and:
Dom0 only: required_features |= dom0_interface
Dom0/DomU: supported_features |= dom0_interface
DomU only: n/a

In the hypervisor/tools, gate the acceptance check on new enough kernel
(i.e., based on whether the kernel has XEN_ELFNOTE_SUPPORTED_FEATURES and/or
XEN_ELFNOTE_REQUIRED_FEATURES, and turn that into a
guest_has_modern_feature_flags boolean which you can integrate into various
acceptance checks).

 -- Keir
>  -- Keir
> 
> 


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

On Thu, 2011-07-21 at 10:34 +0100, Keir Fraser wrote:
> On 21/07/2011 10:10, "Keir Fraser" <keir@xen.org> wrote:
> 
> > On 21/07/2011 10:01, "Jan Beulich"
<JBeulich@novell.com> wrote:
> > 
> >>> My own issue with the unprivileged flag is that I''m
not clear what it
> >>> actually means. When would you *not* set it? I mean it looks
in the Linux
> >>> side you set it unconditionally right now. What''s the
point? Why not remove
> >>> the flag and introduce it when we have good reason and can
attach meaningful
> >>> semantics to it?
> >> 
> >> Again - you''re talking about an actual guest side
implementation (which,
> >> in this particular case, has to honor how the rest of the
implementation
> >> is done, i.e. it has to set the flag unconditionally).
I''m talking about an
> >> abstract interface definition that should suit everyone (existing
as well
> >> as yet to come).
> > 
> > I''m afraid my view is that !dom0 operation is a mode that
every PV guest is
> > capable of -- that''s the definition of what a PV guest is, to
my mind!
> > 
> > Like I said in my other email, pinning it down more precisely -- e.g.,
must
> > have PV drivers -- will have counterexamples.
> > 
> > I don''t know though --- maybe a distro could have dom0 and
domU separate
> > kernel builds, and want to set the flags differently for each. I
wonder how
> > much of a long shot that use case is.
> 
> How about having a XEN_ELFNOTE_REQUIRED_FEATURES as well as
> supported_features? Then you can define just one flag,
> XENFEAT_dom0_interface or somesuch, and:
> Dom0 only: required_features |= dom0_interface
> Dom0/DomU: supported_features |= dom0_interface
> DomU only: n/a
> 
> In the hypervisor/tools, gate the acceptance check on new enough kernel
> (i.e., based on whether the kernel has XEN_ELFNOTE_SUPPORTED_FEATURES
and/or
> XEN_ELFNOTE_REQUIRED_FEATURES, and turn that into a
> guest_has_modern_feature_flags boolean which you can integrate into various
> acceptance checks).
That sounds reasonable to me.

Ian.



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

>>> On 21.07.11 at 11:34, Keir Fraser <keir@xen.org> wrote:
> On 21/07/2011 10:10, "Keir Fraser" <keir@xen.org> wrote:
> 
>> On 21/07/2011 10:01, "Jan Beulich"
<JBeulich@novell.com> wrote:
>> 
>>>> My own issue with the unprivileged flag is that I''m
not clear what it
>>>> actually means. When would you *not* set it? I mean it looks in
the Linux
>>>> side you set it unconditionally right now. What''s the
point? Why not remove
>>>> the flag and introduce it when we have good reason and can
attach meaningful
>>>> semantics to it?
>>> 
>>> Again - you''re talking about an actual guest side
implementation (which,
>>> in this particular case, has to honor how the rest of the
implementation
>>> is done, i.e. it has to set the flag unconditionally). I''m
talking about an
>>> abstract interface definition that should suit everyone (existing
as well
>>> as yet to come).
>> 
>> I''m afraid my view is that !dom0 operation is a mode that
every PV guest is
>> capable of -- that''s the definition of what a PV guest is, to
my mind!
>> 
>> Like I said in my other email, pinning it down more precisely -- e.g.,
must
>> have PV drivers -- will have counterexamples.
>> 
>> I don''t know though --- maybe a distro could have dom0 and
domU separate
>> kernel builds, and want to set the flags differently for each. I wonder
how
>> much of a long shot that use case is.
> 
> How about having a XEN_ELFNOTE_REQUIRED_FEATURES as well as
> supported_features? Then you can define just one flag,
Since there doesn''t seem to be any use of the "required"
marker so far,
I didn''t want to introduce these. But we certainly could (though for
the
moment I''m preparing an updated patch with just the _SUPPORTED_
part).
> XENFEAT_dom0_interface or somesuch, and:
> Dom0 only: required_features |= dom0_interface
> Dom0/DomU: supported_features |= dom0_interface
> DomU only: n/a
Yes, that''s what I basically did now.

Jan
> In the hypervisor/tools, gate the acceptance check on new enough kernel
> (i.e., based on whether the kernel has XEN_ELFNOTE_SUPPORTED_FEATURES
and/or
> XEN_ELFNOTE_REQUIRED_FEATURES, and turn that into a
> guest_has_modern_feature_flags boolean which you can integrate into various
> acceptance checks).
> 
>  -- Keir
> 
>>  -- Keir
>> 
>> 



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