improve definition of `platform.features` #622

AkihiroSuda · 2017-03-21T04:59:55Z

https://github.com/opencontainers/image-spec/blob/ab461b048bd1c8b6077d8e96936f706a518233c2/image-index.md

This OPTIONAL property specifies an array of strings, each specifying a mandatory CPU feature (for example sse4 or aes).

The definition of platform.features seems vague.
At first glance, I thought the values are adopted from Linux /proc/cpuinfo, but I learned that the cpuinfo file never contains sse4.

http://unix.stackexchange.com/questions/43539/what-do-the-flags-in-proc-cpuinfo-mean

Intel-defined CPU features, CPUID level 0x00000001 (ecx)

sse4_1: SSE-4.1

sse4_2: SSE-4.2

More extended AMD flags: CPUID level 0x80000001, ecx

sse4a: SSE-4A

(Note: AMD's SSE-4A is completely unrelated to Intel's SSE4 http://www.cpu-world.com/Glossary/S/SSE4a.html)

How about improving the definition like this:

This OPTIONAL property specifies an array of strings, each specifying a mandatory CPU feature.

Values for features SHOULD use, and implementations SHOULD understand, flags entries listed in the Linux /proc/cpuinfo file. If an architecture is not supported by Linux, it SHOULD be submitted to this specification for standardization.
For example, sse4_1 or aes on the amd64 and i386 architecture.

Also, for compatibility with Docker images, implementations SHOULD understand the following features values as well:

Architecture	Docker-compatible value	OCI standard value
`amd64`, `i386`	`sse4`	`sse4_1`

The text was updated successfully, but these errors were encountered:

jonboulle · 2017-03-21T11:05:08Z

Proposed wording SGTM

wking · 2017-03-21T14:08:44Z

On Tue, Mar 21, 2017 at 04:05:09AM -0700, Jonathan Boulle wrote: Proposed wording SGTM

And to me, although I think we want: If a feature is not supported by Linux… instead of: If an architecture is not supported by Linux… if the idea is to have folks registering things like ‘sse4_2’. And I'd be happier if cpuinfo's flags values were documented somewhere, but I haven't been able to turn up docs yet.

wking · 2017-03-21T14:49:22Z

flags entries listed in the Linux /proc/cpuinfo file.

Unfortunately, flags turns out to be an x86-ism. The x86 code writes it out here, but other arches may do different things. For example, arm uses Features. The best docs I've found so far are in an SO post, which is probably quickly stale and not a good idea as a spec reference ;).

So I'm not sure what to do. Using flags (or Features, etc.) is certainly convenient on Linux, but “dig through the Linux source to find valid values” isn't particularly friendly for validation maintainers, non-Linux implementation maintainers, or non-Linux config authors. On the other hand, the current guidance isn't really helping anyone, so a Linux-x86-specific fix may be an improvement.

stevvooe · 2017-03-21T20:01:09Z

@AkihiroSuda I think you are getting hung up on the examples. If the examples are wrong fix them. There is no "docker compatible" value -- they aren't really in use.

For the most part, these feature flags are platform dependent.

AkihiroSuda · 2017-03-22T01:34:04Z

Thank you all for the feedback.

What should we do toward v1.0?

Plan 1. Mark the platform.features "reserved" for future releases. Implementations SHOULD NOT use this field in v1.0.

Plan 2. Just remove it. We can revive it in future releases if needed.

Plan 3. Standardize it for Linux on well-known platforms

My suggestion is 1 (or 2).

Also, the same discussion could be applied to platform.os.version, platform.os.features and platform.variant, perhaps:

os.version string
This OPTIONAL property specifies the operating system version, for example 10.0.10586.

os.features array of strings
This OPTIONAL property specifies an array of strings, each specifying a mandatory OS feature (for example on Windows win32k).

variant string
This OPTIONAL property specifies the variant of the CPU, for example armv6l to specify a particular CPU variant of the ARM CPU.

cc @RobDolinMS @jstarks @jhowardmsft

AkihiroSuda · 2017-03-27T08:24:27Z

Can we set milestone of this to 1.0.0-rc6?

stevvooe · 2017-03-30T19:36:32Z

platform.features is for x86 CPU features that are required for the image. If other architectures want to use it for specific meanings, there is not reason it couldn't be re-purposed here.

If someone wants to step up and do the work here, that would be great.

Based on Akihiro Suda's proposal [1] and the subsequent discussion. Using the Linux kernel to define the values is very convenient on Linux, but will likely be a pain for folks on other OSes. If a comprehensive, OS-agnostic list is found, we could use that instead, but would want to provide a mapping from that list of values to the Linux cpuinfo slugs for convenience. Providing a mapping from the OS-agnostic list to the cpuinfo analog on non-Linux OSes would also be convenient. Until then, folks wondering what a given feature slug means but unwilling to dig through the Linux source may find the Stack Overflow post at [2] useful. This definition is not only Linux-centric, it is x86-centric. The x86 Linux implementation writes 'flags' out here [3], but arm uses 'Features' [4]. While we could suggest values for other architectures, this commit restricts itself to the x86 family (using their GOARCH names [5], as the runtime spec recommends [6]), because that's the scope Stephen is currently claiming [7]. Extending the field to other architectures can happen in follow-up work. [1]: opencontainers#622 (comment) [2]: http://unix.stackexchange.com/questions/43539/what-do-the-flags-in-proc-cpuinfo-mean [3]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/arch/x86/kernel/cpu/proc.c?id=refs/tags/v4.10.4#n96 [4]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/arch/arm/kernel/setup.c?id=refs/tags/v4.10.4#n1234 [5]: https://golang.org/doc/install/source#environment [6]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc5/config.md#platform [7]: opencontainers#622 (comment) Signed-off-by: W. Trevor King <[email protected]>

wking · 2017-03-31T04:44:26Z

If someone wants to step up and do the work here, that would be great.

There's not much work to do if we agree that scoping the definition to Linux/(386|amd64) is acceptable for now. In #631, I've taken a stab based on @AkihiroSuda's initial proposal with the following changes:

Mention that flags is specific to the x86 family.
Drop the Docker-compat table.
Limit the definition to the x86 family.

I'm also comfortable with @AkihiroSuda's plans 1 or 2, but your response suggests you prefer his plan 3, which is where I've tried to go with #631.

Based on Akihiro Suda's proposal [1] and the subsequent discussion. Using the Linux kernel to define the values is very convenient on Linux, but will likely be a pain for folks on other OSes. If a comprehensive, OS-agnostic list is found, we could use that instead, but would want to provide a mapping from that list of values to the Linux cpuinfo slugs for convenience. Providing a mapping from the OS-agnostic list to the cpuinfo analog on non-Linux OSes would also be convenient. Until then, folks wondering what a given feature slug means but unwilling to dig through the Linux source may find the Stack Overflow post at [2] useful. This definition is not only Linux-centric, it is x86-centric. The x86 Linux implementation writes 'flags' out here [3], but arm uses 'Features' [4]. While we could suggest values for other architectures, this commit restricts itself to the x86 family (using their GOARCH names [5], as the runtime spec recommends [6]), because that's the scope Stephen is currently claiming [7]. Extending the field to other architectures can happen in follow-up work. [1]: opencontainers#622 (comment) [2]: http://unix.stackexchange.com/questions/43539/what-do-the-flags-in-proc-cpuinfo-mean [3]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/arch/x86/kernel/cpu/proc.c?id=refs/tags/v4.10.4#n96 [4]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/arch/arm/kernel/setup.c?id=refs/tags/v4.10.4#n1234 [5]: https://golang.org/doc/install/source#environment [6]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc5/config.md#platform [7]: opencontainers#622 (comment) Signed-off-by: W. Trevor King <[email protected]>

RobDolinMS · 2017-04-13T20:56:13Z

@AkihiroSuda Would you want to convert this to a pull request?

RobDolinMS · 2017-04-13T20:57:48Z

@stevvooe suggested earlier in the discussion that this could be milestone==v1.0.0-rc6. Would one of the @opencontainers/image-spec-maintainers want to set as such?

caniszczyk · 2017-04-13T20:58:36Z

@RobDolinMS done

wking · 2017-04-13T21:03:52Z

On Thu, Apr 13, 2017 at 01:56:14PM -0700, Rob Dolin (MSFT) wrote: @AkihiroSuda Would you want to convert this to a pull request?

This are already two pull requests (#631 and #632).

Based on Akihiro Suda's proposal [1] and the subsequent discussion. Using the Linux kernel to define the values is very convenient on Linux, but will likely be a pain for folks on other OSes. If a comprehensive, OS-agnostic list is found, we could use that instead, but would want to provide a mapping from that list of values to the Linux cpuinfo slugs for convenience. Providing a mapping from the OS-agnostic list to the cpuinfo analog on non-Linux OSes would also be convenient. Until then, folks wondering what a given feature slug means but unwilling to dig through the Linux source may find the Stack Overflow post at [2] useful. This definition is not only Linux-centric, it is x86-centric. The x86 Linux implementation writes 'flags' out here [3], but arm uses 'Features' [4]. While we could suggest values for other architectures, this commit restricts itself to the x86 family (using their GOARCH names [5], as the runtime spec recommends [6]), because that's the scope Stephen is currently claiming [7]. Extending the field to other architectures can happen in follow-up work. The vmx example is based on: On Mon, Apr 24, 2017 at 01:53:43AM -0700, Akihiro Suda wrote [8]: > I think we should not take too much effort for deciding example > values, but how about: > > "feature `vmx` marks support for instruction > `VMXON,VMXOFF,VMLAUNCH,VMRESUME,VMCALL,VMPTRLD,VMPTRST,VMCLEAR,VMREAD > and VMWRITE`, and this image contains an executable compiled to > use `VM... instructions` with no fallback”. > > I see some reasons to deploy such VMs as OCI images: > > - Android emulator (x86) > - Forked versions of KVM, that cannot be installed via apt-get > - `docker run`-nable Unikernel demo https://github.com/Unikernel-Systems/DockerConEU2015-demo > - ... > > For these use cases, checking whether `vmx` supported is useful, > because `vmx` is typically missing on EC2. [1]: opencontainers#622 (comment) [2]: http://unix.stackexchange.com/questions/43539/what-do-the-flags-in-proc-cpuinfo-mean [3]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/arch/x86/kernel/cpu/proc.c?id=refs/tags/v4.10.4#n96 [4]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/arch/arm/kernel/setup.c?id=refs/tags/v4.10.4#n1234 [5]: https://golang.org/doc/install/source#environment [6]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc5/config.md#platform [7]: opencontainers#622 (comment) [8]: opencontainers#631 (comment) Signed-off-by: W. Trevor King <[email protected]>

vbatts · 2017-05-03T19:02:38Z

and #650 #651 #652

stevvooe · 2017-05-08T23:32:04Z

Make sure to see @rneugeba's comment on this matter.

I would move for removing this field altogether.

vbatts · 2017-05-12T18:36:33Z

can we close this then? :-)

wking · 2017-05-12T18:43:00Z

On Fri, May 12, 2017 at 11:36:33AM -0700, v1.0.0.batts wrote: can we close this then? :-)

It should probably stay open until #672 lands.

wking mentioned this issue Mar 31, 2017

image-index: Document Linux cpuinfo flags (on x86) for platform.features #631

Closed

AkihiroSuda mentioned this issue Mar 31, 2017

image-index: mark os.version, os.features and variant as "reserved" #632

Closed

caniszczyk added this to the v1.0.0-rc6 milestone Apr 13, 2017

stevvooe mentioned this issue May 11, 2017

image-index: remove platform.features field #672

Merged

stevvooe mentioned this issue May 12, 2017

image-index: define platform.variant for arm #650

Merged

vbatts closed this as completed in #672 May 19, 2017

AkihiroSuda mentioned this issue Feb 14, 2019

Support OCI Image spec field Platform.features moby/moby#38715

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve definition of `platform.features` #622

improve definition of `platform.features` #622

AkihiroSuda commented Mar 21, 2017

jonboulle commented Mar 21, 2017

wking commented Mar 21, 2017 via email

wking commented Mar 21, 2017 •

edited

Loading

stevvooe commented Mar 21, 2017

AkihiroSuda commented Mar 22, 2017 •

edited

Loading

AkihiroSuda commented Mar 27, 2017

stevvooe commented Mar 30, 2017

wking commented Mar 31, 2017

RobDolinMS commented Apr 13, 2017

RobDolinMS commented Apr 13, 2017

caniszczyk commented Apr 13, 2017

wking commented Apr 13, 2017 via email •

edited

Loading

vbatts commented May 3, 2017

stevvooe commented May 8, 2017

vbatts commented May 12, 2017

wking commented May 12, 2017 via email

improve definition of platform.features #622

improve definition of platform.features #622

Comments

AkihiroSuda commented Mar 21, 2017

jonboulle commented Mar 21, 2017

wking commented Mar 21, 2017 via email

wking commented Mar 21, 2017 • edited Loading

stevvooe commented Mar 21, 2017

AkihiroSuda commented Mar 22, 2017 • edited Loading

AkihiroSuda commented Mar 27, 2017

stevvooe commented Mar 30, 2017

wking commented Mar 31, 2017

RobDolinMS commented Apr 13, 2017

RobDolinMS commented Apr 13, 2017

caniszczyk commented Apr 13, 2017

wking commented Apr 13, 2017 via email • edited Loading

vbatts commented May 3, 2017

stevvooe commented May 8, 2017

vbatts commented May 12, 2017

wking commented May 12, 2017 via email

improve definition of `platform.features` #622

improve definition of `platform.features` #622

wking commented Mar 21, 2017 •

edited

Loading

AkihiroSuda commented Mar 22, 2017 •

edited

Loading

wking commented Apr 13, 2017 via email •

edited

Loading