Skip to content

quartus-prime-lite: improvements & fixes#257089

Merged
bjornfor merged 14 commits intoNixOS:masterfrom
bjornfor:quartus-improvements
Oct 13, 2023
Merged

quartus-prime-lite: improvements & fixes#257089
bjornfor merged 14 commits intoNixOS:masterfrom
bjornfor:quartus-improvements

Conversation

@bjornfor
Copy link
Contributor

@bjornfor bjornfor commented Sep 24, 2023

Description of changes

Best reviewed per commit:

  • quartus-prime-lite: run mkdir as needed
  • quartus-prime-lite: lower case local shell variables
  • quartus-prime-lite: list progs to wrap in sh instead of Nix
  • quartus-prime-lite: add /lib/ld-lsb*.so.3 dynamic loaders to FHS env
  • quartus-prime-lite: add lmutil
  • quartus-prime-lite: add vcom, vdel, vmap
  • quartus-prime-lite: expose all of quartus/bin/*
  • quartus-prime-lite: eliminate two unneeded execve syscalls
  • quartus-prime-lite: don't overwrite LD_PRELOAD
  • quartus-prime-lite: buildFHSEnvChroot -> buildFHSEnv

UPDATE: More commits:

  • quartus-prime-lite: modelsim: fix compiling encrypted device models
  • quartus-prime-lite: test building encrypted device model
  • quartus-prime-lite: use runtimeShell in wrappers

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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 basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 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.

Instead of maintaining both a list of executables to wrap and their
directory names, extract the directory names from the executables and
run mkdir as needed. Keep DRY.
Upper case shell variables are generally for internal variables (SHELL,
BASH_VERSION) or environment variables (PAGER, EDITOR). Other variables
should be lower case.
This opens up for using sh globbing (and possibly wrapping *all*
programs), something which is not possible when the program list is
coded in Nix.
(In preparation for adding lmutil.)

Quartus is a mix of 32- and 64-bit programs, and these "lsb" loaders are
required by some of the unwrapped binaries:

  $ find /nix/store/HASH-quartus-prime-lite-unwrapped-20.1.1.720 -type f -executable | xargs -n1 patchelf --print-interpreter |& grep "ld-lsb" | sort -u
  /lib64/ld-lsb-x86-64.so.3
  /lib/ld-lsb.so.3
It's tool to check the validity of licenses, and vsim suggests running
it when it has issues validating a license. (At least in Quartus
22.1.2.)
These are useful for Modelsim scripting.
* Upstream recommends it (adding $QUARTUS_ROOTDIR/bin to $PATH).
* It's cool that nixpkgs *can* do these tricks (only expose a subset of
  programs), but in this case I don't think it's a good idea. For
  example, before this change I was missing the `jtagconfig` program.
* This increases the number of programs in .../bin from 29 to 80.
* Set LD_PRELOAD in the profile snippet to eliminate one execve() for `env`.
* Set runScript to "" to eliminate one execve() for bash.
Instead, merge the user provided LD_PRELOAD environment variable with
the hardcoded libudev.so.1 entry. User provided libs are loaded first.
The former is deprecated. The latter is implemented with
buildFHSEnvBubblewrap. The reason this package was switched to
buildFHSEnvChroot in the first place[1] is fixed by passing `multiArch =
true` (which brings e.g. 32-bit dynamic loader /lib/ld-linux.so.2).

[1] Commit cae417d ("quartus-prime: use buildFHSEnvChroot")
@bjornfor bjornfor marked this pull request as ready for review September 24, 2023 13:46
@bjornfor bjornfor requested a review from kwohlfahrt September 24, 2023 13:46
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Sep 24, 2023
@bjornfor bjornfor requested review from Mic92 and tpwrules September 27, 2023 06:40
LD_PRELOADing libudev breaks compiling encrypted device models in
ModelSim, so only use LD_PRELOAD for non-ModelSim wrappers.

Before:

  $ "$(NIXPKGS_ALLOW_UNFREE=1 nix-build -A quartus-prime-lite)/bin/vlog" "$(NIXPKGS_ALLOW_UNFREE=1 nix-build -A quartus-prime-lite.unwrapped)/modelsim_ase/altera/verilog/src/arriav_atoms_ncrypt.v"
  [...]
  ** Error: /nix/store/szcr2q24izqvhz7ybalar43y5xdg172a-quartus-prime-lite-unwrapped-20.1.1.720/modelsim_ase/altera/verilog/src/arriav_atoms_ncrypt.v(38): (vlog-2163) Macro `<protected> is undefined.
  ** Error: /nix/store/szcr2q24izqvhz7ybalar43y5xdg172a-quartus-prime-lite-unwrapped-20.1.1.720/modelsim_ase/altera/verilog/src/arriav_atoms_ncrypt.v(38): (vlog-2163) Macro `<protected> is undefined.
  ** Error: (vlog-13069) /nix/store/szcr2q24izqvhz7ybalar43y5xdg172a-quartus-prime-lite-unwrapped-20.1.1.720/modelsim_ase/altera/verilog/src/arriav_atoms_ncrypt.v(38): syntax error in protected region.
  [...]
  Errors: 4, Warnings: 0

After:

  $ "$(NIXPKGS_ALLOW_UNFREE=1 nix-build -A quartus-prime-lite)/bin/vlog" "$(NIXPKGS_ALLOW_UNFREE=1 nix-build -A quartus-prime-lite.unwrapped)/modelsim_ase/altera/verilog/src/arriav_atoms_ncrypt.v"
  [...]
  Errors: 0, Warnings: 0
stdenv.shell is a shell for building, runtimeShell is for running, so
the latter should be used in wrappers. (The distinction only matters
when cross-compiling.)
@bjornfor
Copy link
Contributor Author

bjornfor commented Oct 9, 2023

Merging in two days unless there are objections.

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Looks fine visually. Didn't test it.

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 10, 2023
Copy link
Contributor

@kwohlfahrt kwohlfahrt left a comment

Choose a reason for hiding this comment

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

I don't have time to test, but this looks good to me.

wrapper=$out/bin/${name}
progs_wrapped=()
for prog in ''${progs_to_wrap[@]}; do
relname="''${prog#"${unwrapped}/"}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not up on my bash-isms, this is doing something like realpath --relative-to "${unwrapped}" "$prog"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, looks like it.

@delroth delroth added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Oct 10, 2023
@tpwrules
Copy link
Contributor

I will test later today

@tpwrules
Copy link
Contributor

tpwrules commented Oct 10, 2023

qsys-edit needs dejavu_fonts added to its requirements in order to launch and gnumake in order to generate HDL, that needs to be addressed so that works properly. Not a new problem but worth finally fixing.

Some other suggestions you might or might not consider taking:

  1. I have made this change to allow Quartus to run under e.g. QEMU binfmt_misc emulation.
  2. A "negative interaction" between older kernels and current coreutils causes the cp which moves the installers into the temporary directory to fail on my system. I had to replace with cat [src] > [dst] to fix. Might be nice to patch here until coreutils gets patched and makes it into nixpkgs.

Other than that, I was able to generate a design with Qsys then synthesize it with Quartus both as GUI apps and in the Nix sandbox. I was also able to USB blast my design onto a board. I did not check Modelsim.

@bjornfor
Copy link
Contributor Author

qsys-edit needs dejavu_fonts added to its requirements in order to launch and gnumake in order to generate HDL, that needs to be addressed so that works properly. Not a new problem but worth finally fixing.

I'm not familiar with that tool, but the GUI comes up (with seemingly working fonts) both on nixpkgs-unstable and on this PR branch. I suggest making further imrovements in other PRs.

Good to merge or does anyone want to test more?

@tpwrules
Copy link
Contributor

I'm not familiar with that tool, but the GUI comes up (with seemingly working fonts) both on nixpkgs-unstable and on this PR branch. I suggest making further imrovements in other PRs.

All this should need is to add the two packages under # qsys requirements in targetPkgs.

@bjornfor
Copy link
Contributor Author

I'm not familiar with that tool, but the GUI comes up (with seemingly working fonts) both on nixpkgs-unstable and on this PR branch. I suggest making further imrovements in other PRs.

All this should need is to add the two packages under # qsys requirements in targetPkgs.

I see, but since I don't know how to test it, and qsys-edit already launch for me, I'd prefer if that can be done in another PR. (You also mentioned other fixes (qemu), which I don't feel comfortable with including here just because it's related to quartus.)

@bjornfor bjornfor merged commit 99476c1 into NixOS:master Oct 13, 2023
@bjornfor bjornfor deleted the quartus-improvements branch October 13, 2023 15:21
@bjornfor bjornfor changed the title quartus-prime-lite: many improvements quartus-prime-lite: improvements & fixes Oct 13, 2023
@github-actions
Copy link
Contributor

Successfully created backport PR for release-23.05:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants