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

AkihiroSuda · 2017-04-24T08:25:40Z

Please refer to #632 for previous discussion.

In this PR, I avoided to use Linux-specific uname values for ease of porting to non-Linux.
#632 (comment)

Signed-off-by: Akihiro Suda [email protected]

AkihiroSuda · 2017-04-24T08:57:05Z

CI failed but locally passes
https://travis-ci.org/opencontainers/image-spec/builds/225111253

$ make lint
checking lint
WARNING: deadline exceeded by linter staticcheck on schema (try increasing --deadline)
WARNING: deadline exceeded by linter errcheck on schema (try increasing --deadline)
make: *** [lint] Error 2
The command "make lint" exited with 2.

wking · 2017-04-24T16:38:17Z

image-index.md

@@ -63,7 +63,9 @@ For the media type(s) that this document is compatible with, see the [matrix][ma

    - **`variant`** *string*

-        This OPTIONAL property specifies the variant of the CPU, for example `armv6l` to specify a particular CPU variant of the ARM CPU.
+        This OPTIONAL property specifies the variant of the CPU.
+        When `architecture` is `arm`, values for arch SHOULD use, and runtimes SHOULD understand, arch entries listed in the Go Language document for [`$GOARM`](go-arm). i.e. `5`, `6`, or `7`.


“values for arch” → “values for variant”. And maybe “arch entries” → “architecture versions” to match Go's wording.

mattspencer-arm · 2017-04-27T14:21:38Z

schema/backwards_compatibility_test.go

@@ -100,8 +100,7 @@ func TestBackwardsCompatibilityImageIndex(t *testing.T) {
         "digest": "sha256:fb2fc0707b86dafa9959fe3d29e66af8787aee4d9a23581714be65db4265ad8a",
         "platform": {
            "architecture": "arm64",
-            "os": "linux",
-            "variant": "armv8"


I would like to see "variant": "8" here for completeness.

I don't think we should add it until upstream Go adds "8".

Note that there is no guarantee Go adds "8".
It can be "8-foo" depending on the future extension, or maybe completely different name depending on the future ARM versioning scheme.

A question then - is the OCI specification subordinate to go with respect to the value of attributes here? Would this not make the implementation of this specification sensitive to changes in go?
I thought that the metadata in this specification was to define the compatibility of the images - in which case we ought to be complete in our definition here. The full definition in this instance would be { "arch": "arm64", "variant": "8" }?

@stevvooe WDYT?

another idea is to use Linux utsname (armv8l, armv7l) across all the platforms, although Linux-specific #632 (comment)

I have been chatting with the kernel/toolchain and disto teams here within ARM, and the consensus is that there is currently no reliable way of runtime detecting the underlying architecture.

To quote our kernel team when asked about runtime detection
"As an example, Linux reports ARMv7 for ARM11MPCore or ARM1176 (and I think newer revisions of ARM1136). The architecture versions are just some milestones or new baselines without a dedicated ID. The architected CPUID registers only expose features and not architecture versions. However, since some features are "back-ported" to previous
architecture versions, the kernel gets confused (the ARMv7 MMU features have been built into ARM11MPCore, hence Linux reporting v7).

The "armv8l" is only reported by an AArch64 kernel when running a task with the PER_LINUX32 personality. Otherwise the utsname is "aarch64". If you run an AArch32 kernel on ARMv8, it will report armv7l.

I doubt the above helps but I think we should just avoid such run-time
detection at the architecture version. If the hardware does not provide
this information, the OS cannot reliably infer it."

I know this is not very helpful at the moment, but it is a statement of where we are today. There is talk about ARM potentially investing time in 'lscpu' with the help of a LUT to correctly identify architecture.

The full definition in this instance would be { "arch": "arm64", "variant": "8" }?

This example was only effort put forth on this particular pull request. The actual usage current looks like this:

{ "mediaType": "application/vnd.docker.distribution.manifest.v1+json", "size": 2084, "digest": "sha256:07ebe243465ef4a667b78154ae6c3ea46fdb1582936aac3ac899ea311a701b40", "platform": { "architecture": "arm", "os": "linux", "variant": "armv7" } }, { "mediaType": "application/vnd.docker.distribution.manifest.v1+json", "size": 2090, "digest": "sha256:fb2fc0707b86dafa9959fe3d29e66af8787aee4d9a23581714be65db4265ad8a", "platform": { "architecture": "arm64", "os": "linux", "variant": "armv8" } }

mattspencer-arm · 2017-04-27T14:23:04Z

schema/backwards_compatibility_test.go

@@ -91,7 +91,7 @@ func TestBackwardsCompatibilityImageIndex(t *testing.T) {
         "platform": {
            "architecture": "arm",


I understand that "arm" is what gets expressed by GOARCH, which is why it is used here. But would it not be better to disambiguate the property by calling it "arm32"?

I am not sure if this is the right place to add this, but I also think that for completeness we ought to have arch entries for "arm32", "arm64" and "arm64ilp32"?

This is defined by runtime-spec, and I don't think we can change anymore.
https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc3/config.md#platform

That's fine, and as a result of a historical decisions.
ARM would rather see the use of arm32 to reference their 32 bit instruction set rather than the company name - which could be considered ambiguous?

ARM would rather see the use of arm32 to reference their 32 bit instruction set rather than the company name - which could be considered ambiguous?

Please take this up with the Go team.

stevvooe · 2017-04-28T19:16:57Z

After looking into this further, mapping variant to GOARM is incorrect. The variant field must have the abi+isa within it. For example, arm64ipl32 would be the variant.

AkihiroSuda · 2017-05-02T19:04:21Z

@mattspencer-arm PTAL about @stevvooe 's comment?
#650 (comment)

Also, I think it would be better to add an implementator's note about how to check supported abi and isa.
With regarding to arm64ilp32, https://wiki.linaro.org/Platform/arm64-ilp32 might be useful, but couldn't get what is the standard way

AkihiroSuda · 2017-05-12T22:11:56Z

Can we remove and reserve this field as in platform.features #672?

cc @estesp

AkihiroSuda · 2017-05-12T22:17:49Z

Uh, I just received email from stevvooe to make this forward, +1

https://groups.google.com/a/opencontainers.org/forum/m/#!topic/dev/Eht-i5UbUvo

stevvooe · 2017-05-12T22:22:28Z

@AkihiroSuda Was just coming here to post that!

stevvooe · 2017-05-12T22:24:22Z

@AkihiroSuda Would you mind updating this PR in the vein of that proposal? They are quite compatible, but I'd like to create a table, as is in the proposal. I'll paste that table into #661.

AkihiroSuda · 2017-05-14T09:47:16Z

done

stevvooe · 2017-05-15T19:39:09Z

image-index.md

+        | ISA/ABI                              | `architecture` | `variant`   |
+        |--------------------------------------|----------------|-------------|
+        | ARM 32-bit, v5 and earlier           | Unsupported    | Unsupported |
+        | ARM 32-bit, v6 (non-Raspberry Pi 1)  | Unsupported    | Unsupported |


Leave these lines off. Nothing is explicitly unsupported.

I agree - but I would replace 'unsupported' with 'out of scope' to make it explicit that this spec does not cover older ARM ISAs.

stevvooe · 2017-05-15T19:39:34Z

image-index.md

+        |--------------------------------------|----------------|-------------|
+        | ARM 32-bit, v5 and earlier           | Unsupported    | Unsupported |
+        | ARM 32-bit, v6 (non-Raspberry Pi 1)  | Unsupported    | Unsupported |
+        | ARM 32-bit, v6 (Raspberry Pi 1)      | `arm`          |`v6`         |


Leave "Raspberry Pi 1". That was just a note in the proposal.

Signed-off-by: Akihiro Suda <[email protected]>

AkihiroSuda · 2017-05-16T01:59:57Z

done

mattspencer-arm · 2017-05-16T08:22:57Z

Can I suggest that we don't use the word 'Unsupported' but replace it with 'Out of scope'. I think this better reflects the position? /Matt

…

________________________________ From: Akihiro Suda <[email protected]> Sent: 16 May 2017 02:59:59 To: opencontainers/image-spec Cc: Matt Spencer; Mention Subject: Re: [opencontainers/image-spec] image-index: define platform.variant for arm (#650) done — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#650 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/Aayrr8Eawjq4HBWHjGc02vhsPZp3Tp8xks5r6QMfgaJpZM4NF1ta>. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

stevvooe · 2017-05-16T20:49:10Z

Can I suggest that we don't use the word 'Unsupported' but replace it with 'Out of scope'. I think this better reflects the position?

I don't think these lines are even needed. If someone wants to build ARM support for pre-v6, they are within their right, but it won't be specified and if it conflicts, it conflicts.

@AkihiroSuda Let's just remove those lines.

AkihiroSuda · 2017-05-17T01:35:10Z

already removed

hqhq · 2017-05-17T09:28:16Z

image-index.md

+        | ARM 32-bit, v6  | `arm`          | `v6`        |
+        | ARM 32-bit, v7  | `arm`          | `v7`        |
+        | ARM 32-bit, v8  | `arm`          | `v8`        |
+        | ARM 64-bit, v8  | `arm64`        | `v8`        |


If we scope arch and variant with this table, and remove features, we can't identify platforms with ilp32 support, are there other issues tracing this?

And is variant really ARM specific property?

Please see https://golang.org/src/go/build/syslist.go for extra architecture values. I think ilp32 will be arm64p32. It is not here because it has low adoption at this point.

And is variant really ARM specific property?

It is not. Where did you see this implied?

Would it be possible to include a v5 variant? armv5/armel is still supported by go, and would be great to cover all options (and it's in use for some devices at us at resin.io)

stevvooe · 2017-05-17T21:37:50Z

LGTM

vbatts · 2017-05-17T21:47:16Z

LGTM

AkihiroSuda mentioned this pull request Apr 24, 2017

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

Closed

AkihiroSuda mentioned this pull request Apr 24, 2017

.tool/lint: increase deadline #653

Merged

wking reviewed Apr 24, 2017

View reviewed changes

wking mentioned this pull request Apr 24, 2017

image-index: define platform.os.features on Windows #652

Merged

AkihiroSuda force-pushed the platform-variant branch from 6666beb to b911488 Compare April 25, 2017 06:27

wking mentioned this pull request Apr 27, 2017

FINAL v1.0 RELEASE BURN DOWN #615

Closed

2 tasks

mattspencer-arm reviewed Apr 27, 2017

View reviewed changes

wking mentioned this pull request Apr 28, 2017

Identifying ARM platforms #661

Closed

vbatts mentioned this pull request May 3, 2017

improve definition of platform.features #622

Closed

AkihiroSuda force-pushed the platform-variant branch 2 times, most recently from 8e0e38b to a818d88 Compare May 14, 2017 09:45

stevvooe reviewed May 15, 2017

View reviewed changes

image-index: define platform.variant for arm

4d4320f

Signed-off-by: Akihiro Suda <[email protected]>

AkihiroSuda force-pushed the platform-variant branch from a818d88 to 4d4320f Compare May 16, 2017 01:59

hqhq reviewed May 17, 2017

View reviewed changes

stevvooe added this to the v1.0.0-rc6 milestone May 17, 2017

vbatts merged commit 8cd3190 into opencontainers:master May 17, 2017

vbatts mentioned this pull request May 19, 2017

Bump version to rc6 #681

Merged

tianon mentioned this pull request May 30, 2017

Refactor the "multiarch" section of the README to make it clear these things are now "officially supported" (in many cases) docker-library/official-images#2949

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

AkihiroSuda commented Apr 24, 2017

AkihiroSuda commented Apr 24, 2017

wking Apr 24, 2017

AkihiroSuda Apr 25, 2017

mattspencer-arm Apr 27, 2017

AkihiroSuda Apr 28, 2017

mattspencer-arm Apr 28, 2017

AkihiroSuda Apr 28, 2017

AkihiroSuda Apr 28, 2017

mattspencer-arm Apr 28, 2017

stevvooe May 2, 2017

mattspencer-arm Apr 27, 2017 •

edited

Loading

AkihiroSuda Apr 28, 2017

mattspencer-arm Apr 28, 2017

stevvooe May 2, 2017

stevvooe commented Apr 28, 2017

AkihiroSuda commented May 2, 2017

AkihiroSuda commented May 12, 2017

AkihiroSuda commented May 12, 2017

stevvooe commented May 12, 2017

stevvooe commented May 12, 2017 •

edited

Loading

AkihiroSuda commented May 14, 2017

stevvooe May 15, 2017

mattspencer-arm May 16, 2017

stevvooe May 15, 2017

AkihiroSuda commented May 16, 2017

mattspencer-arm commented May 16, 2017 via email

stevvooe commented May 16, 2017

AkihiroSuda commented May 17, 2017

hqhq May 17, 2017

stevvooe May 17, 2017

imrehg May 31, 2017

stevvooe commented May 17, 2017 •

edited by caniszczyk

Loading

vbatts commented May 17, 2017 •

edited by caniszczyk

Loading

		@@ -91,7 +91,7 @@ func TestBackwardsCompatibilityImageIndex(t *testing.T) {
		"platform": {
		"architecture": "arm",

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

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

Conversation

AkihiroSuda commented Apr 24, 2017

AkihiroSuda commented Apr 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattspencer-arm Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented Apr 28, 2017

AkihiroSuda commented May 2, 2017

AkihiroSuda commented May 12, 2017

AkihiroSuda commented May 12, 2017

stevvooe commented May 12, 2017

stevvooe commented May 12, 2017 • edited Loading

AkihiroSuda commented May 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda commented May 16, 2017

mattspencer-arm commented May 16, 2017 via email

stevvooe commented May 16, 2017

AkihiroSuda commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented May 17, 2017 • edited by caniszczyk Loading

vbatts commented May 17, 2017 • edited by caniszczyk Loading

mattspencer-arm Apr 27, 2017 •

edited

Loading

stevvooe commented May 12, 2017 •

edited

Loading

stevvooe commented May 17, 2017 •

edited by caniszczyk

Loading

vbatts commented May 17, 2017 •

edited by caniszczyk

Loading