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

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Tue, 2011-07-19 at 11:24 +0100, Jan Beulich wrote:
> >>> On 19.07.11 at 11:28, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel