-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
internal/cpu: support darwin/arm64 CPU feature detection [freeze exception] #42747
Comments
from my MacBook Pro M1. |
I'd like to request a freeze exception for this. Without hardware support, AES is not only extremely slow, but also not constant time, which is a security issue. crypto/tls will switch away from it, but not all applications can dynamically fallback, and not all peers support good alternatives. The implementation is Go 1.16 would have to be scoped to only darwin/arm64 (not replacing the one in the other OSes), but if it is I think it would have very limited chances of affecting other targets, and darwin/arm64 will inevitably be in flux during the freeze anyway. /cc @rsc @golang/release |
Can we just set |
A low complexity workaround since there is only one Apple Silicon Chip (except the DTK which should be similar and testable) is that for Then for go1.17 we can follow up with runtime CPU feature checks. Im working on a CL to test the hard coding of CPU features. |
We have to consider that Go 1.16 will have to work correctly until 2022, and while I don't expect new Apple Silicon chips to drop such core features, we don't really know in advance. Binaries built with Go 1.16 might also stick around longer than Go 1.16 itself, and this is logic that would end up embedded in all of them. |
Change https://golang.org/cl/271986 mentions this issue: |
I see these options to prevent regressions with future
Note that we likely need an allowlist for |
@FiloSottile Thanks for letting us know. We've discussed this on the release team, and based on the rationale you provided and the external constraints that could not be worked around in advance, we are leaning towards this freeze exception being granted. Please keep the release schedule in mind and re-evaluate this as needed if this cannot be resolved without causing a significant additional delay or risk for Go 1.16. Update: Approved. |
Looking further how to have runtime feature detection in
UPDATE: I have it working on darwin/amd64. Now need to wire it up for the posted arm64 sysctl names and then somebody needs to test it. |
Change https://golang.org/cl/271989 mentions this issue: |
Since we still have some hardcoded CPU features (AES) for which no What is the output of On amd64 Mac OS X I see:
|
Nothing (Mac Mini M1):
|
I’ll go a step further and suggest that you can assume the presence of these instructions not just for Go 1.16 but for all time. There shouldn’t be any need for run-time detection of these features on mac-arm64. For that matter, although ios-arm64 has a lower baseline CPU than mac-arm64, these specific instructions are also guaranteed to be available on ios-arm64 as well. In support, I offer clang and llvm, which in code contributed by Apple engineers:
In further support, none of the instructions under consideration ( Similarly, even for some of the |
…and I’ll put my money where my mouth is: I assume the presence of The only reason I don’t assume We’re also assuming ARMv8.3 as a baseline, for example via our use of The clang/llvm references above support all of this, and in fact, if you’re using Apple clang (as opposed to open-source, which hasn’t caught up), you’ll even find the |
@markmentovai Thanks for the information. I think https://go-review.googlesource.com/c/go/+/271989/7/src/internal/cpu/cpu_arm64_darwin.go that I came up with before your comment aligns with your comments. Go already assumes all I do not think we need to assume |
If this lands, similar changes should be made to x/sys/cpu as it suffers from the same issue. Should I open a new issue for that? |
I find the arguments for none of these features being optional pretty convincing, and this late in the cycle I think the smaller CL 271986 would be a safer bet. If Apple's LLVM is happy assuming those features, we're fine doing the same. |
In BoringSSL (and thus crypto in Chromium), we also use static armcaps for Apple ARM64 platforms. We look at My impression so far (prior to ARM macOS) was that they largely used static armcaps, dispatching with universal binaries and the app store. Now they're on another generation of aarch64 features and macOS has an ARM port, it makes sense that they'd would shift more to (The joys of ARM's platform-specific feature detection...) |
I've edited an earlier #42747 (comment) to clarify this freeze exception has been approved. |
Created a separate issue for |
darwin/arm64
does not support CPUID or HWCAP CPU feature detection like other operating systems on arm64 currently supported by Go.A new way using sysctlbyname will need to be implemented to detect if for example
hw.optional.armv8_1_atomics
is set to 1.https://developer.apple.com/documentation/apple_silicon/addressing_architectural_differences_in_your_macos_code
If someone has an M1 Apple Silicon Mac please post the output of:
sysctl -a | grep "hw\.optional"
The text was updated successfully, but these errors were encountered: