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

[wking]

Based on @AkihiroSuda's proposal and the subsequent discussion (#622).

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 this Stack Overflow post useful.

This definition is not only Linux-centric, it is x86-centric. The x86 Linux implementation writes flags out here, but arm uses Features. While we could suggest values for other architectures, this commit restricts itself to the x86 family (using their GOARCH names, as the runtime spec recommends), because that's the scope @stevvooe is currently claiming. Extending the field to other architectures can happen in follow-up work.

[stevvooe]

These should not be defined in terms of linux, but instead in terms of amd64/x86.

[wking]

These should not be defined in terms of linux, but instead in terms of amd64/x86.

Isn't that what I'm doing? As I said in the initial post, I'm happy to link to something other than a Linux header if we find an OS-agnostic registry, but I'm currently not aware of one.

[jonboulle]

oops, forgot to click submit

[jonboulle]

perhaps "on Linux on these architectures"

[wking]

Done with ee92880 → dc5e4a0.

[jonboulle]

"When architecture is set to 386 or amd64, image indexes SHOULD use"

don't we also need to clarify that os == linux ?

[wking]

don't we also need to clarify that os == linux ?

The features are architecture-specific, and are OS-independent. It's just our current definition of strings that is tied to Linux, and I will happily change that to use an OS-agnostic registry of CPU features if we can find one (as discussed in the initial post).

[jonboulle]

unset or set to any other value

[wking]

unset or set to any other value

I think “neither 386 nor amd64” already covers architecture being unset or set to another value, and can't come up with clearer wording that incorporates your phrase. Can you just paste out your whole recommended sentence in case I'm missing something obvious?

[jonboulle]

@wking should/can we close this in favour of #632?

[wking]

On Mon, Apr 03, 2017 at 10:34:28AM -0700, Jonathan Boulle wrote: @wking should/can we close this in favour of #632?

If that's easier for maintainers, I'm happy to do that [1]. However, I like the wording here more than what's currently in #632, so if we close this I'll advocate for: * A cpufeatures.h reference (which allows maintainers interested in non-Linux OSes to still discover the feature slugs they should be supporting). * Making ‘features’ implementation-defined (instead of unspecified) on non-x86-ish architectures, so users can figure out what their tooling is trying to do, even if that tooling doesn't register their values with the image spec. Putting that discussion in #632 along with its other threads starts to feel crowded to me, but if it won't bog down #632 I'm happy to move the conversation over there. [1]: #632 (comment)

[AkihiroSuda]

I removed duplicating part from my PR.

[AkihiroSuda]

diff --git a/image-index.md b/image-index.md
index 977aa80..c1c9778 100644
--- a/image-index.md
+++ b/image-index.md
@@ -69,10 +69,10 @@ For the media type(s) that this document is compatible with, see the [matrix][ma
 
         This OPTIONAL property specifies an array of strings, each specifying a mandatory CPU feature.
 
-        When `architecture` is 386 or amd64 Image indexes SHOULD use, and implementations SHOULD understand, values [supported by Linux][cpufeatures.h] with the `X86_FEATURE_` pref
ix removed and the remainder lowercased (e.g. `fpu` for `X86_FEATURE_FPU` and `vme` for `X86_FEATURE_VME`).
+        When `architecture` is `386` or `amd64`, image indexes SHOULD use, and implementations SHOULD understand, values [supported by Linux][cpufeatures.h] with the `X86_FEATURE_`
 prefix removed and the remainder lowercased (e.g. `fpu` for `X86_FEATURE_FPU` and `vme` for `X86_FEATURE_VME`).
         On Linux on these architectures, the features supported by host CPUs can be found in the `flags` entries in the `cpuinfo` file provided by the [proc filesystem][proc.5].
 
-        When `architecture` is neither 386 nor amd64, values are implementation-defined.
+        When `architecture` is neither `386` nor `amd64`, it SHOULD be submitted to this specification for standardization.
 
 - **`annotations`** *string-string map*

[wking]

@AkihiroSuda, in dc5e4a0 → e1045bf, I've picked up your backticks, “Image” → “image”, and commas. I've also picked up your “SHOULD be submitted”, but kept my “implementation-defined” for the reasons I gave here.

[AkihiroSuda]

For L108 (GitHub doesn't allow me to directly comment there)

• os.features -> features
• sse4 -> sse4_2 (please grep and update other files as well)

[wking]

Fixed “os.features” → “features” in dc5e4a0 → e1045bf. Also replaced the old sse4 instances, although I replaced them with fpu since I was using fpu and vme since those are the first two entries in cpufeatures.h. I don't really care what features we use as examples though; if there's a strong preference for sse4_2, I can switch to that instead.

[AkihiroSuda]

cc @xiaochenshen (author of some Xeon-specific runtime spec: opencontainers/runtime-spec@73a6002)

[AkihiroSuda]

fpu and vme are valid as example, but seems weird because almost all the processors would implement them.

How about rather using rdt_a and cat_l3, which are required to use the feature standardized in image-spec: opencontainers/runtime-spec@73a6002

[xiaochenshen]

@AkihiroSuda

How about rather using rdt_a and cat_l3, which are required to use the feature standardized in image-spec: opencontainers/runtime-spec@73a6002

Sounds good to me.

More explanation of rdt_a and cat_l3 flags in /proc/cpuinfo:
rdt_a: Allocation sub-features of Intel Resource Director Technology (RDT). Cache Allocation Technology (CAT) is one of the features.
cat_l3: CAT is a sub-feature of Intel RDT, L3 cache is the last level cache for allocation in some Intel Xeon CPU platforms.

For more information about Intel RDT and CAT:
opencontainers/runc#433

[wking]

cat_l3: CAT is a sub-feature of Intel RDT...

So is it possible to have cat_l3 but not rtd_a? I'm trying to understand if we need to list both, or if listing a cat_l3 requirement robustly covers the rdt_a requirement too.

Also, this sort of resource allocation seems like more of a runtime concern than an image concern. An image that requires CAT is hard to imagine. Maybe it's going to be the only container on my host, in which case I think I should be able to run it even if my processor doesn't support CAT (with a runtime config that doesn't call for CAT).

[xiaochenshen]

@wking

So is it possible to have cat_l3 but not rtd_a? I'm trying to understand if we need to list both, or if listing a cat_l3 requirement robustly covers the rdt_a requirement too.

cat_l3 depends on rdt_a. If the flag cat_l3 is available in /proc/cpuinfo, rdt_a should be also available in /proc/cpuinfo.

Logically:
rdt_a = cache allocation (cat) + other shared resource allocation, e.g., memory bandwidth allocation (mba, in some Intel Xeon CPU based platforms in future)

cat = L3 cache allocation (cat_l3) or L2 cache allocation (cat_l2, in some Intel Atom CPU based platforms in future)

Also, this sort of resource allocation seems like more of a runtime concern than an image concern. An image that requires CAT is hard to imagine. Maybe it's going to be the only container on my host, in which case I think I should be able to run it even if my processor doesn't support CAT (with a runtime config that doesn't call for CAT).

I agree with you that the resource allocation is more of a runtime concern. It looks like hardware-assisted cgroup for resource (e.g., L3 cache) constraints.

You can refer to opencontainers/runc#1279
For OCI/runc, the runtime config for CAT is valid only when both hardware (CPU) and kernel support CAT feature.

In following cases, L3 cache is entirely shared. It is equivalent to the case which we don't have CAT feature.
(1) We don't specify CAT runtime config
(2) CAT is not supported by hardware
(3) CAT kernel driver is not available

[wking]

On Tue, Apr 11, 2017 at 10:49:55AM -0700, Xiaochen Shen wrote: cat_l3 depends on rdt_a. If the flag cat_l3 is available in /proc/cpuinfo, rdt_a should be also available in /proc/cpuinfo.

So if you were setting platform.features for this, you'd only include cat_l3 to stay DRY.

I agree with you that the resource allocation is more of a runtime concern.

So can we find some features that are more likely to be image concerns? It seems like the most defensible would be “feature $FEAT marks support for instruction $INST, and this image contains an executable compiled to use $INST with no fallback”. And ideally this would be backed up by an actual image that someone is using. If we can't find such an image, we may want to consider reserving this field for now. The ‘fpu’ feature doesn't quite meet those criteria, because there are likely to be software workarounds if the CPU doesn't support floating point operations. But if you're doing anything that requires a lot of floating point calculations, you *really* want hardware support for them, so it's not that far off. And while broad support for ‘fpu’ [1] makes the example less interesting, I think an image designed for lots of floating point operations should still list ‘fpu’ in platform.features. I'm not familiar enough with ‘vme’ to know, but expect it falls into the same boat as ‘fpu’. [1]: #631 (comment)

[AkihiroSuda]

@wking

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.

[wking]

On Mon, Apr 24, 2017 at 01:53:43AM -0700, Akihiro Suda wrote: - `docker run`-nable Unikernel demo https://github.com/Unikernel-Systems/DockerConEU2015-demo

This is great, a real-world example that needs this feature :). I've updated the wording to only use vmx as an example with e1045bf → e6136dc. How does that look to you?

[AkihiroSuda]

fpu -> vmx

[wking]

Thanks. Fixed with e6136dc → ca8c1fd.

[AkihiroSuda]

SGTM (IANAM)

[vbatts]

is this ready for review?

[wking]

On Wed, May 03, 2017 at 12:41:33PM -0700, v1.0.0.batts wrote: is this ready for review?

Is there a threshold for that? ;). I'm not aware of any outstanding change requests, at any rate. If anybody has something they'd like me to change that I've forgotten about, please remind me :).

[rn]

I'm not sure it is a good idea to specify CPU features for images and then match them with host features, mostly for practical reasons.

Every new Intel architecture revision introduces new features. On my system I currently have about 60+ features reported. The Linux kernel needs to detect these new feature and report them. Some of this detection/reporting code is not necessarily backported to older kernels. So, you may have a new CPU but your vendor kernel does not report the features it supports, preventing you from running some images although your hardware supports them. In VMs it's even harder because your Hypervisor also needs to detect the features, so you need a matching between Hypervisor, Guest OS kernel and images flags (and you Hypervisor may needs special support for some features).

How do you determine which flags to set on your image? Compiler flags are typically different to cpufeatures.h flags. They are also typically more abstract, like "Compile for this specific processor architecture". And of course the flags differ even between C compilers, never mind compilers for other languages. And then you still have no idea which features are actually used by the compiler. I see no practical way for me to specify which features to set for my image.

And this is just the "simple" Intel/x86/linux case...

My suggestion would be to either not have features field at all or use it purely advisory, more like a hint, so that a warning can be printed, or, if a image crashes with an illegal instruction exception, one can report an error and say, "hey, maybe it's because some of these features don't match".

[wking]

On Thu, May 04, 2017 at 01:11:00AM -0700, Rolf Neugebauer wrote: Some of this detection/reporting code is not necessarily backported to older kernels. So, you may have a new CPU but your vendor kernel does not report the features it supports, preventing you from running some images although your hardware supports them.

That's a fair point, but as it stands in ca8c1fd there is no language about what image-spec implementations should *do* with these values. This PR is only defining what values are valid, and (sort of) what those values mean. So if you need a particular instruction (and your binary has no auto-detection or fallback), you set the appropriate flag in ‘features’. If a particular host is interested in your image and the CPU supports that instruction, you can use it regardless of whether that host's kernel reports the feature or not. I'm open to adding a caveat to this effect to the current wording. How does: On Linux on these architectures, the features supported by host CPUs can be found in the `flags` entries in the `cpuinfo` file provided by the [proc filesystem][proc.5], although older kernels may not report all features supported by newer CPUs. sound to you?

How do you determine which flags to set on your image?

You could get an upper limit by scanning the binaries for instructions: $ objdump -d /bin/busybox | sed -n 's/^.*\t.*\t\([^ ]*\) .*/\1/p' | sort | uniq and then mapping from instruction to feature flag (e.g. AESENC → aes). But that's not going to pick up cases where the binary includes a fallback for an unsupported instruction. But I expect that in most cases, features entries will be added because: 1. The software authors declare required features, and the image authors (who may also be the software authors) carry those dev-declared features into their image. 2. A user tries to run the image on a CPU that does not support a required instruction, they report the failure to the image authors, and the image authors add the missing feature listing when updating their image.

And then you still have no idea which features are actually used by the compiler.

This is the main issue which separates the ‘features’ field from compiler options. Unless you dig enough to figure this out, you may have to rely on the second approach (patching features as required instructions are reported).

My suggestion would be to either not have `features` field at all or use it purely advisory, more like a hint, so that a warning can be printed, or, if a image crashes with an illegal instruction exception, one can report an error and say, "hey, maybe it's because some of these features don't match".

The current wording allows this advisory approach. If you feel it does not, what would you suggest I change to allow the advisory approach?

[stevvooe]

@wking I've been on both sides of the scenarios you are hypothesizing. You either have to compile for both machines, update the old machines or compile for the lowest common denominator. The combinatoric explosion is unrealistic to handle in such a simple field. In reality, a full solution for a scenario where this crosses the cost-benefit threshold would be highly specialized to the hardware and deployment of that hardware. For applications where this is valuable, careful labeling would be much more fruitful and can handle the myriad possibility that is possible to encounter.

After looking into this problem closely, my current recommendation is to completely remove or deprecate the field. It was my suggestion, long ago, in the creation of the manifest list, and I apologize to the community for ever conceiving of the idea.

Rather than wasting valuable man-years in trying to specify this and deal with confusion, the much wiser approach here is to cut losses and remove it.

In that regard, I move that we close this PR, in favor of removing the field. @opencontainers/image-spec-maintainers

[stevvooe]

I have submitted to #672.

[wking]

Rather than wasting valuable man-years in trying to specify this and deal with confusion, the much wiser approach here is to cut losses and remove it.

Sounds good to me :). This is @AkihiroSuda's plan 2, which I'm happy with.