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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

armTrustedFirmwareAllwinner: Add missing build flags #299116

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kreyren
Copy link
Contributor

@Kreyren Kreyren commented Mar 26, 2024

Adds a missing build flags needed for OLIMEX Teres-1 (and likely other sun50i_a64 devices) to setup regulators and power-management to avoid various issues such as disfunctional display.

Fixes: #297358

Things done

  • Built on platform(s)
    • x86_64-linux -- Compiled via binfmt for aarch64-linux
    • aarch64-linux -- Cross-compiled from x86_64-linux appears to have upstream-induced issues, not recommended
    • [-] x86_64-darwin
    • [-] aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • [-] sandbox = relaxed
    • [-] sandbox = true
  • [-] Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage -- Tested as a part of uboot and tested on the device
  • Tested basic functionality of all binary files (usually in ./result/bin/) -- Tested on OLIMEX Teres-1 and used as a daily device with
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • [-] (Package updates) Added a release notes entry if the change is major or breaking
    • [-] (Module updates) Added a release notes entry if the change is significant
    • [-] (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 馃憤 reaction to pull requests you find important.

Adds a missing build flags needed for OLIMEX Teres-1 (and likely other sun50i_a64 devices) to setup regulators and power-management to avoid various issues such as disfunctional display.

Fixes: NixOS#297358
@lopsided98
Copy link
Contributor

This doesn't make sense to me. I don't see any evidence that Nix is somehow setting these flags to zero. When I build with V=1, I can see that -DSUNXI_PSCI_USE_NATIVE=1 -DSUNXI_PSCI_USE_SCPI=1 -DSUNXI_SETUP_REGULATORS=1 is passed to every compiler invocation. Lastly, I get bit for bit identical binaries with or without this patch.

@lopsided98
Copy link
Contributor

Cross-compiled from x86_64-linux appears to have upstream-induced issues, not recommended

I can't reproduce this either. My cross and native builds are bit for bit identical.

All of these builds (cross, native, with and without this patch) using commit 05c97b5, produce a bl31.bin with SHA256 6acf48db6a1bbc4fa9cef485d2829a86462ccf649d59f1273d652a008ac24a50

@Kreyren
Copy link
Contributor Author

Kreyren commented Mar 27, 2024

This doesn't make sense to me. I don't see any evidence that Nix is somehow setting these flags to zero. When I build with V=1, I can see that -DSUNXI_PSCI_USE_NATIVE=1 -DSUNXI_PSCI_USE_SCPI=1 -DSUNXI_SETUP_REGULATORS=1 is passed to every compiler invocation. Lastly, I get bit for bit identical binaries with or without this patch. -- @lopsided98 (#299116 (comment))

On my end it's reproducible that without these flags it appears to always have broken display initialization and overall being in a very broken state where the device is very slow to respond to commands.

Tested as 2/2 times without the flags and 2/2 times with the flags, device appears to work flawlessly with the flags.

I can try to do more tests to verify.

@Kreyren
Copy link
Contributor Author

Kreyren commented Mar 27, 2024

I can't reproduce this either. My cross and native builds are bit for bit identical.

All of these builds (cross, native, with and without this patch) using commit 05c97b5, produce a bl31.bin with SHA256 6acf48db6a1bbc4fa9cef485d2829a86462ccf649d59f1273d652a008ac24a50 -- @lopsided98 (#299116 (comment))

It's expected to build and generate a binary without an issue, what i mentioned there is possible upstream bug that i am trying to fix on the device which is reported from across linux distros where uboot shows various issues likely when it's cross-compiled. It's possible that it's related to atf which is why it's mentioned in case this contribution was tested on the hardware, but currently it's likely an issue in crust or somewhere else. (no issue tracking yet as i didn't yet figure out possible cause)

@Kreyren Kreyren marked this pull request as draft March 27, 2024 07:39
@lopsided98
Copy link
Contributor

There must be some other confounding factor causing the behavior you observe. This PR on its own definitely does nothing.

I tested with the following script:

let
  build = nixpkgs: let
    pkgsNative = import nixpkgs { system = "aarch64-linux"; };
    pkgsCross = (import nixpkgs { }).pkgsCross.aarch64-multiplatform;
    
    bl31Hash = pkg: {
      out = "${pkg}";
      hash = builtins.hashFile "sha256" "${pkg}/bl31.bin";
    };
    ubootHash = pkg: {
      out = "${pkg}"; 
      hash = builtins.hashFile "sha256" "${pkg}/u-boot-sunxi-with-spl.bin";
    };
  in {
    "armTrustedFirmwareAllwinner-native" = bl31Hash pkgsNative.armTrustedFirmwareAllwinner;
    "armTrustedFirmwareAllwinner-cross" = bl31Hash pkgsCross.armTrustedFirmwareAllwinner;
    "ubootOlimexA64Teres1-native" = ubootHash pkgsNative.ubootOlimexA64Teres1;
    "ubootOlimexA64Teres1-cross" = ubootHash pkgsCross.ubootOlimexA64Teres1;
  };
in {
  "upstream" = build (builtins.fetchTarball {
    url = "https://github.com/NixOS/nixpkgs/archive/756377102b367156c5a515d02dd51d4e8af33e16.tar.gz";
    sha256 = "sha256:1nmv16rzd22lzdab4jvakdqvgv4a3vadbcb1311p88pap6mzhq0i";
  });
  "pr-299116" = build (builtins.fetchTarball {
    url = "https://github.com/NixOS/nixpkgs/archive/d0b331787e47e302f7ec34d7a6a1a1ad91f3171f.tar.gz";
    sha256 = "sha256:1ngd44f56mvhn6v64gnmqxy83aa0g01zck80crk7ngs6g3bbz3gk";
  });
}

If I save this file as teres-atf.nix and then run nix eval --json -f teres-atf.nix | jq, I get the following output:

{
  "pr-299116": {
    "armTrustedFirmwareAllwinner-cross": {
      "hash": "6acf48db6a1bbc4fa9cef485d2829a86462ccf649d59f1273d652a008ac24a50",
      "out": "/nix/store/kgg5l04r6kkpq3iqsy3c2j2apj8bsfs6-arm-trusted-firmware-sun50i_a64-aarch64-unknown-linux-gnu-2.10.0"
    },
    "armTrustedFirmwareAllwinner-native": {
      "hash": "6acf48db6a1bbc4fa9cef485d2829a86462ccf649d59f1273d652a008ac24a50",
      "out": "/nix/store/9ziyg4ymx9k259va9dlpq5pqqjq482km-arm-trusted-firmware-sun50i_a64-2.10.0"
    },
    "ubootOlimexA64Teres1-cross": {
      "hash": "4d63bb2c20dfb46241e466327902c4430fd176979f6f542558cf3126b0610b34",
      "out": "/nix/store/98j4bq3r1g9hqqfsspxrn516yykh2lyi-uboot-teres_i_defconfig-aarch64-unknown-linux-gnu-2024.01"
    },
    "ubootOlimexA64Teres1-native": {
      "hash": "671ee68771c4ecebecfd643d4789dffd8f8a9b4c2acc10c37a1ccba7c3afb6db",
      "out": "/nix/store/kqfd3nzrdwar0smlfraadvqflbkxh61i-uboot-teres_i_defconfig-2024.01"
    }
  },
  "upstream": {
    "armTrustedFirmwareAllwinner-cross": {
      "hash": "6acf48db6a1bbc4fa9cef485d2829a86462ccf649d59f1273d652a008ac24a50",
      "out": "/nix/store/iid06h9cx5sq18za7039imid1amsrz8j-arm-trusted-firmware-sun50i_a64-aarch64-unknown-linux-gnu-2.10.0"
    },
    "armTrustedFirmwareAllwinner-native": {
      "hash": "6acf48db6a1bbc4fa9cef485d2829a86462ccf649d59f1273d652a008ac24a50",
      "out": "/nix/store/w4q3azzc02swxkjmhfs6gd0zlkbzqhll-arm-trusted-firmware-sun50i_a64-2.10.0"
    },
    "ubootOlimexA64Teres1-cross": {
      "hash": "4d63bb2c20dfb46241e466327902c4430fd176979f6f542558cf3126b0610b34",
      "out": "/nix/store/7gnn5pq8xzmgwbyw45xl2712wrrj8300-uboot-teres_i_defconfig-aarch64-unknown-linux-gnu-2024.01"
    },
    "ubootOlimexA64Teres1-native": {
      "hash": "671ee68771c4ecebecfd643d4789dffd8f8a9b4c2acc10c37a1ccba7c3afb6db",
      "out": "/nix/store/pxa7xpnwpkzvgmys6m186vrslb908xi9-uboot-teres_i_defconfig-2024.01"
    }
  }
}

This shows that the binaries produced with and without this PR are identical. There are differences between the native and cross U-Boot builds, but that is not the relevant issue here.

@Kreyren
Copy link
Contributor Author

Kreyren commented Mar 28, 2024

Thanks for looking into it, i don't know what's happening here i get that the binaries are the same, but if these flags are not set it has issues (5/5 reproducible)

investigating..

EDIT: Apparently happens on random and i was lucky enough to observe the behavior as said, identified as uboot bug will need SETUP_SUNXI_REGULATORS=0 once addresed in uboot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants