Skip to content
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

Add FreeBSD Arm64 support #295

Merged
merged 10 commits into from
Sep 19, 2023
Merged

Add FreeBSD Arm64 support #295

merged 10 commits into from
Sep 19, 2023

Conversation

toor1245
Copy link
Contributor

@toor1245 toor1245 commented Feb 1, 2023

Getting all the features is handled by reading /var/run/dmesg.boot. Feature detections were taken from the freebsd kernel code sys/arm64/arm64/identcpu.c. This PR does not contain MIDR_EL1 and cache function definitions. For future detection midr we can try to use mapping of CPU 0 string to MIDR_EL1 values: https://github.com/freebsd/freebsd-src/blob/main/sys/arm64/arm64/identcpu.c#L173, but not in this PR since the changes are so big

refs to review:
https://github.com/freebsd/freebsd-src/tree/main/sys/arm64/arm64

@toor1245 toor1245 force-pushed the freebsd-arm64 branch 2 times, most recently from 420d9fc to fddcfaf Compare February 1, 2023 01:55
test/cpuinfo_aarch64_test.cc Outdated Show resolved Hide resolved
Comment on lines 271 to 289
// TODO: Add sveebf16 detection
ID_AA64ZFR0_FEATURES(sveebf16, AARCH64_SVEBF16)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 256 to 268
// TODO: Add ssbs2 detection
ID_AA64PFR1_FEATURES(ssbs2, AARCH64_SSBS)
Copy link
Contributor Author

@toor1245 toor1245 Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speculative Store Bypassing controls in AArch64 state. Defined values are:

SSBS Meaning
0b0000 AArch64 provides no mechanism to control the use of Speculative Store Bypassing.
0b0001 AArch64 provides the PSTATE.SSBS mechanism to mark regions that are Speculative Store Bypass Safe.
0b0010 AArch64 provides the PSTATE.SSBS mechanism to mark regions that are Speculative Store Bypassing Safe, and the MSR and MRS instructions to directly read and write the PSTATE.SSBS field.

FEAT_SSBS implements the functionality identified by the value 0b0001.
FEAT_SSBS2 implements the functionality identified by the value 0b0010.

in current impl linux identifies ssbs2 as ssbs:

HWCAP_SSBS
Functionality implied by ID_AA64PFR1_EL1.SSBS == 0b0010.

refs:

so we should add ssbs2 new field, set to true ssbs for linux when ID_AA64PFR1_EL1.SSBS == 0b0010 is true, for freebsd just add here new AARCH64_SSBS2

Comment on lines 228 to 232
ID_AA64ISAR1_SET_FEATURE(api_pac, "API PAC"),
ID_AA64ISAR1_SET_FEATURE(api_epac, "API EPAC"),
ID_AA64ISAR1_SET_FEATURE(api_epac2, "Impl PAuth+EPAC2"),
ID_AA64ISAR1_SET_FEATURE(api_fpac, "Impl PAuth+FPAC"),
ID_AA64ISAR1_SET_FEATURE(api_fpac_combined, "Impl PAuth+FPAC+Combined"),
Copy link
Contributor Author

@toor1245 toor1245 Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linux HWCAP idenifies only 0b0001 cases:

HWCAP_PACA
Functionality implied by ID_AA64ISAR1_EL1.APA == 0b0001 or ID_AA64ISAR1_EL1.API == 0b0001, as described by Pointer authentication in AArch64 Linux.

HWCAP_PACG
Functionality implied by ID_AA64ISAR1_EL1.GPA == 0b0001 or ID_AA64ISAR1_EL1.GPI == 0b0001, as described by Pointer authentication in AArch64 Linux.

we can add new fields and detect it in Linux as well via check HWCAP_CPUID, if this value is true, we can get EL1 info in userland, for example, pacg get info via inline asm

refs:

@gchatelet
Copy link
Collaborator

Thx a lot for the great PR ! That's quite some work 😄 I especially like the detailed sources.

AFAIU most of the code is inspired from https://github.com/freebsd/freebsd-src/tree/main/sys/arm64/arm64.

A few questions though:

  • It seemed to me that MIDR was only accessible in kernel space but from your first comment it seems that a part of it is reachable from userland. Correct?
  • If so would it make sense to move the reusable part in a separate header to also use it in Linux?

@toor1245
Copy link
Contributor Author

Thx a lot for the great PR ! That's quite some work 😄 I especially like the detailed sources.

AFAIU most of the code is inspired from https://github.com/freebsd/freebsd-src/tree/main/sys/arm64/arm64.

A few questions though:

  • It seemed to me that MIDR was only accessible in kernel space but from your first comment it seems that a part of it is reachable from userland. Correct?
  • If so would it make sense to move the reusable part in a separate header to also use it in Linux?

correct, we can read privilegied MIDR_EL1 (implementer, part, revision, architecture) or ID_AA64_* system register info from userspace but from certain versions:

it's worth noting that FreeBSD supports arm64 since release/11.0.0 (8 years ago), and as example Go runtime supports arm64 only from release/12.0.0 (5 years ago) https://github.com/golang/go/wiki/MinimumRequirements#freebsd.

All raspberry pi versions supports from 12.0.0:
https://wiki.freebsd.org/arm/Raspberry%20Pi

So, we can move arm64 logic to separate file like x86 and get cpu info via assembly and reuse the same code for FreeBSD and Linux.

Advantages:

  • Common logic
  • Get more information and consistent detetection (when cpuid is supported) at least on these operating systems
  • Easy to add tests

Disadvantages:

  • Support for the old implementation (parsing /var/run/dmesg.boot, /proc/cpuinfo, hwcaps) and the new one

About parsing CPU 0, if this approach of the current PR suits us, then for the definition of implementer, part (MIDR_EL1) we can use info from tables to map data to integer:
https://github.com/freebsd/freebsd-src/blob/main/sys/arm64/arm64/identcpu.c#L174
https://github.com/freebsd/freebsd-src/blob/main/sys/arm64/arm64/identcpu.c#L235
https://github.com/freebsd/freebsd-src/blob/main/sys/arm64/arm64/identcpu.c#L2070

@gchatelet
Copy link
Collaborator

Highly appreciate the thorough documentation (as always).
I think it's worth factoring the common logic then.

I would be tempted to land this patch first and work on a refactoring as a second step. WDYT?

@pkubaj
Copy link

pkubaj commented Feb 23, 2023

On FreeBSD you need to use elf_aux_info() to get CPU capabilities. It works similarly to Linux's getauxval(). Parsing /var/run/dmesg.boot is wrong.

@toor1245
Copy link
Contributor Author

@pkubaj, I didn't know about this function, thanks. However, this function since 12.0.0 (https://man.freebsd.org/cgi/man.cgi?query=elf_aux_info&sektion=3&format=html) as well as reading system registers from userland.

@gchatelet, what do you think?

@gchatelet
Copy link
Collaborator

Looking at FreeBSD history; 12.0.0 is available from early 2019. It's quite recent.

We'll probably have to go the linux way and get data from elf_aux_info if available but fall back to /var/run/dmesg.boot parsing otherwise...

@pkubaj do you know what was the official way to get cpu features in userland for FreeBSD pre 12.0.0?

@pkubaj
Copy link

pkubaj commented Feb 23, 2023

I wouldn't call it recent. FreeBSD 12.3-RELEASE is the oldest version currently supported. FreeBSD 12 as a whole goes EOL this year. That said, the recommended way for older versions was something like:

unsigned long hwcaps2 = 0;
size_t len = sizeof(hwcaps2);
sysctlbyname("hw.cpu_features2", &hwcaps2, &len, NULL, 0);

That was for AT_HWCAP2.

@toor1245
Copy link
Contributor Author

toor1245 commented Feb 25, 2023

@pkubaj, @gchatelet, I checked sysctlbyname hw.cpu_features, hw.cpu_features2, unfortunately, it doesn't work for arm64. elf_aux_info - works fine.

I just leave here part of sysctl hw (FreeBSD 14.0-CURRENT RPI4):

hw.machine: arm64
hw.model: ARM Cortex-A72 r0p3
hw.ncpu: 4
hw.byteorder: 1234
hw.physmem: 8420352000
hw.usermem: 7730343936
hw.pagesize: 4096
hw.machine_arch: aarch64
hw.realmem: 8443125760
hw.cpufreq.temperature: 35499
hw.cpufreq.voltage_sdram_p: 1100000
hw.cpufreq.voltage_sdram_i: 1100000
hw.cpufreq.voltage_sdram_c: 1100000
hw.cpufreq.voltage_core: 850000
hw.cpufreq.turbo: 0
hw.cpufreq.sdram_freq: 400000000
hw.cpufreq.core_freq: 200000000
hw.cpufreq.arm_freq: 600000000

Looks like hw.cpu_features call accessible only for powerpc

so, I think we worth to use proposed @gchatelet approach, if no objections, but I have to update current impl as ID_AA64ZFR0_EL1 identification code is not exists in 11 version and FP is Float and check again the fields in version 11 whether anything else has changed:

https://github.com/freebsd/freebsd-src/blob/stable/11/sys/arm64/arm64/identcpu.c#L717-L724
https://github.com/freebsd/freebsd-src/blob/stable/11/sys/arm64/arm64/identcpu.c#L324

Other fields that HWCAP does not provide, we can use with ID_AA64_* EL1 from 12.0.0 version for FreeBSD and Linux.

what do you think?

@gchatelet
Copy link
Collaborator

Ok let's go with elf_aux_info and fallback to /var/run/dmesg.boot otherwise.
It's nice that there is now an official way to get cpu features under FreeBSD even if it's still incomplete.

@toor1245 toor1245 force-pushed the freebsd-arm64 branch 2 times, most recently from 04dd437 to 58478d4 Compare March 12, 2023 21:43
@toor1245 toor1245 marked this pull request as draft March 19, 2023 10:04
@toor1245
Copy link
Contributor Author

toor1245 commented Sep 1, 2023

@gchatelet, I have question related to fallback, I checked the first supported version of Arm64 and it is 11 version and this version is Tier 2 (Developmental and Niche Architectures), since 13 version Arm64 is Tier 1 (Fully-Supported Architectures). What do you think to use only elf_aux_info (supported from 12 version) without fallback?

also, 11-STABLE is EOL
I think we can do like Go runtime, they started support from 12 version https://github.com/golang/go/wiki/MinimumRequirements#freebsd

refs:
https://docs.freebsd.org/en/articles/committers-guide/#_statement_of_general_intent
https://www.freebsd.org/platforms/

@gchatelet
Copy link
Collaborator

@toor1245 SGTM let's drop support for 11 and go with 12. Thx!

Mykola Hohsdze and others added 6 commits September 5, 2023 20:14
Getting all the features is handled by reading /var/run/dmesg.boot. Feature detections were taken from the freebsd kernel code sys/arm64/arm64/identcpu.c
@toor1245 toor1245 force-pushed the freebsd-arm64 branch 4 times, most recently from 41fb37a to e454ffe Compare September 14, 2023 22:20
@toor1245 toor1245 force-pushed the freebsd-arm64 branch 3 times, most recently from 545b2f2 to c94a331 Compare September 15, 2023 02:43
@toor1245 toor1245 force-pushed the freebsd-arm64 branch 3 times, most recently from afe7840 to 6408bda Compare September 15, 2023 22:43
@toor1245
Copy link
Contributor Author

fix clang format for hwcaps.h, see #340

@toor1245 toor1245 force-pushed the freebsd-arm64 branch 2 times, most recently from 9d6f18a to 699bb0c Compare September 15, 2023 23:25
@toor1245 toor1245 force-pushed the freebsd-arm64 branch 2 times, most recently from cc6242c to 7fc1bfd Compare September 17, 2023 12:43
@toor1245 toor1245 marked this pull request as ready for review September 17, 2023 13:11
@toor1245
Copy link
Contributor Author

toor1245 commented Sep 17, 2023

@gchatelet, @Mizux, this PR is ready to review. Few notes that you should know

1) The root shell is tcsh(1) by default on FreeBSD 13 and earlier and sh(1) on FreeBSD 14 and later. So for Bazel user can get the following message if try to run without bash

mhohsadze@generic:~/cpu_features_toor1245_2 $ bazel run list_cpu_features
INFO: Analyzed target //:list_cpu_features (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:list_cpu_features up-to-date:
  bazel-bin/list_cpu_features
INFO: Elapsed time: 0.735s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/list_cpu_features
FATAL: execv of '/usr/local/bin/bash' failed: (error: 2): No such file or directory

To fix this issue I installed pkg bash

pkg install bash
mhohsadze@generic:~/cpu_features_toor1245_2 $ bazel run list_cpu_features
INFO: Analyzed target //:list_cpu_features (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:list_cpu_features up-to-date:
  bazel-bin/list_cpu_features
INFO: Elapsed time: 0.991s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/list_cpu_features
arch            : aarch64
implementer     :   0 (0x00)
variant         :   0 (0x00)
part            :   0 (0x00)
revision        :   0 (0x00)
flags           : asimd,crc32,fp

ref: https://docs.freebsd.org/en/articles/linux-users/#shells

2) It looks like googletest with Bazel doesn't work on FreeBSD, there is known issue that Bazel use clang instead of clang++

bazelbuild/bazel#12023
bazelbuild/rules_cc#161
google/googletest#3878

I got the following error message:

mhohsadze@generic:~/cpu_features_toor1245_2 $ bazel run cpuinfo_test
DEBUG: Rule 'com_google_googletest' indicated that a canonical reproducible form can be obtained by modifying arguments commit = "e2239ee6043f73722e7aa812a459f54a28552929" and dropping ["tag"]
DEBUG: Repository com_google_googletest instantiated at:
  /home/mhohsadze/cpu_features_toor1245_2/WORKSPACE:5:15: in <toplevel>
Repository rule git_repository defined at:
  /home/mhohsadze/.cache/bazel/_bazel_mhohsadze/c0c856c8d68586bbc248a05faeb84cd1/external/bazel_tools/tools/build_defs/repo/git.bzl:181:33: in <toplevel>
INFO: Analyzed target //:cpuinfo_test (2 packages loaded, 174 targets configured).
INFO: Found 1 target...
INFO: From Compiling test/cpuinfo_aarch64_test.cc:
test/cpuinfo_aarch64_test.cc:77:24: warning: unused function 'cpu' [-Wunused-function]
static FakeCpuAarch64& cpu() {
                       ^
1 warning generated.
ERROR: /home/mhohsadze/cpu_features_toor1245_2/BUILD.bazel:343:8: Linking cpuinfo_test failed: (Exit 1): clang failed: error executing command (from target //:cpuinfo_test) /usr/bin/clang -o bazel-out/freebsd-fastbuild/bin/cpuinfo_test -Xlinker -rpath -Xlinker '$ORIGIN/_solib_freebsd/' -Xlinker -rpath -Xlinker ... (remaining 15 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
ld: error: bazel-out/freebsd-fastbuild/bin/_solib_freebsd/libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so: undefined reference to nextafter [--no-allow-shlib-undefined]
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Target //:cpuinfo_test failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 108.718s, Critical Path: 58.37s
INFO: 44 processes: 11 internal, 33 processwrapper-sandbox.
FAILED: Build did NOT complete successfully
ERROR: Build failed. Not running target

3) I noticed a strange thing that I can read system registers like MIDR_EL1 in user mode but elf_aux_info HWCAP_CPUID from AT_HWCAP reports that I can't do this.

#include <sys/auxv.h>
#include <stdio.h>
#include <stdint.h>

static uint64_t
extract_bit_range(uint64_t reg, uint64_t msb, uint64_t lsb)
{
        const uint64_t bits = msb - lsb + 1ULL;
        const uint64_t mask = (1ULL << bits) - 1ULL;
        return ((reg >> lsb) & mask);
}

uint64_t
get_midr_el1()
{
        uint64_t midr_el1;
        __asm("mrs %0, MIDR_EL1" : "=r"(midr_el1));
        return midr_el1;
}

int
main()
{
        unsigned long hwcaps;
        uint64_t midr_el1;

        elf_aux_info(AT_HWCAP, &hwcaps, sizeof(unsigned long));
        printf("hwcap_cpuid: %lu\n", hwcaps & HWCAP_CPUID);

        midr_el1 = get_midr_el1();

        printf("midr_el1: 0x%lx\n", midr_el1);
        printf("implementer: 0x%lx\n", extract_bit_range(midr_el1, 31, 24));
        printf("variant: 0x%lx\n", extract_bit_range(midr_el1, 23, 20));
        printf("part: 0x%lx\n", extract_bit_range(midr_el1, 15, 4));
        printf("revision: 0x%lx\n", extract_bit_range(midr_el1, 3, 0));
        return 0;
}

Output:

hwcap_cpuid: 0
midr_el1: 0x410fd083
implementer: 0x41
variant: 0x0
part: 0xd08
revision: 0x3

I checked identcpu.c and I did not find the location of HWCAP_CPUID where the value of elf_hwcap is set, only MRS_HWCAP for system registers.

that's why it reports values as 0, since info.features.cpuid = 0.

Aarch64Info GetAarch64Info(void) {
  // ...
  if (info.features.cpuid) {
    const uint64_t midr_el1 = GetMidrEl1();
    info.implementer = (int)ExtractBitRange(midr_el1, 31, 24);
    info.variant = (int)ExtractBitRange(midr_el1, 23, 20);
    info.part = (int)ExtractBitRange(midr_el1, 15, 4);
    info.revision = (int)ExtractBitRange(midr_el1, 3, 0);
  }
  return info;
}

So I wrote to one of the FreeBSD Arm64 committers to confirm this issue.

  1. AT_PLATFORM and AT_BASE_PLATFORM doesn't exist in FreeBSD at all, see
    https://github.com/freebsd/freebsd-src/blob/main/sys/sys/elf_common.h#L953-L989, so I return just NULL for these function CpuFeatures_GetPlatformPointer(void), CpuFeatures_GetBasePlatformPointer(void), and probably it can be improved to remove these extra functions in FreeBSD impl but I think it should be done as separate patch.

@toor1245
Copy link
Contributor Author

toor1245 commented Sep 17, 2023

@gchatelet, @Mizux, It would also be great to add Bazel instructions and known issues to README as separate patch.
What do you think?

@gchatelet
Copy link
Collaborator

Thx for the PR @toor1245, I'll look it up today.

@gchatelet gchatelet self-assigned this Sep 19, 2023

PLATFORM_OS_ANDROID = ("@platforms//os:android")

PLATFORM_OS_FREEBSD = ("@platforms//os:freebsd")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this may look weird that we have to provide this file and not use @platforms//... directly but this comes from the fact that we have internal definitions for these cpu/platforms that are different from upstream Bazel. I hope we'll be able to remove this technical debt one day.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the tests will have to be split at some point. It's starting to be too complicated. Not in this PR but definitely in the near future.

@gchatelet
Copy link
Collaborator

Thx again for the hard work here. Let's merge and iterate if needed before the next release.

@gchatelet gchatelet merged commit 89a3f03 into google:main Sep 19, 2023
29 checks passed
@toor1245
Copy link
Contributor Author

@gchatelet, I got confirmation that it was a bug, it was fixed couple days ago, see
https://cgit.freebsd.org/src/commit/?id=d61f9bfb0e5c119c97a559f187b1e9c73077307b

@toor1245
Copy link
Contributor Author

I did build with the latest changes from main branch and can confirm that it works fine!

mhohsadze@generic:~/cpu_features_toor1245_2 $ bazel run list_cpu_features
Starting local Bazel server and connecting to it...
... still trying to connect to local Bazel server (1532) after 10 seconds ...
INFO: Analyzed target //:list_cpu_features (37 packages loaded, 192 targets configured).
INFO: Found 1 target...
Target //:list_cpu_features up-to-date:
  bazel-bin/list_cpu_features
INFO: Elapsed time: 61.431s, Critical Path: 4.52s
INFO: 8 processes: 1 internal, 7 processwrapper-sandbox.
INFO: Build completed successfully, 8 total actions
INFO: Running command line: bazel-bin/list_cpu_features
arch            : aarch64
implementer     :  65 (0x41)
variant         :   0 (0x00)
part            : 3336 (0xD08)
revision        :   3 (0x03)
flags           : asimd,cpuid,crc32,fp
mhohsadze@generic:~/cpu_features_toor1245_2 $ 

So, just to note that it works for now only in main branch

A user with a stable version of FreeBSD will see the result I posted recently:

mhohsadze@generic:~/cpu_features_toor1245_2 $ bazel run list_cpu_features
INFO: Analyzed target //:list_cpu_features (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:list_cpu_features up-to-date:
  bazel-bin/list_cpu_features
INFO: Elapsed time: 0.991s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/list_cpu_features
arch            : aarch64
implementer     :   0 (0x00)
variant         :   0 (0x00)
part            :   0 (0x00)
revision        :   0 (0x00)
flags           : asimd,crc32,fp

@gchatelet
Copy link
Collaborator

Thx a lot for taking the time to contact the team and report here.
Maybe we should create an issue with this specific bug so people running into it can pinpoint the root cause more easily?

@toor1245
Copy link
Contributor Author

toor1245 commented Oct 3, 2023

Thx a lot for taking the time to contact the team and report here. Maybe we should create an issue with this specific bug so people running into it can pinpoint the root cause more easily?

@gchatelet, yes I agree with you that we should create an issue

@toor1245
Copy link
Contributor Author

@gchatelet, I added issue #344

@gchatelet
Copy link
Collaborator

Thx a lot @toor1245 🙏 , I meant to do it but I've been busy.

@toor1245 toor1245 mentioned this pull request Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants