Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,6 @@ The following system packages are required: `qemu-system-x86`, `ovmf`, `mtools`,
- `pf` (the task runner) is NOT a pip package. It is cloned from `https://github.com/P4X-ng/pf-runner` and the `pf-cli-base/pf_parser.py` file is installed as `~/.local/bin/pf` with shebang `#!/usr/bin/env python3`. It requires `fabric` and `lark` pip packages.
- `$HOME/.local/bin` must be on `PATH` for pip-installed scripts and the `pf` runner to work.
- The cloud VM does not have `/dev/kvm`, so QEMU tests must omit `-enable-kvm` and `-cpu host`.
- The cloud VM kernel does not support `vfat` mounts, so `pf build-package-esp` and `pf test-qemu` fail out-of-the-box. Build the ESP image manually using `mtools` (`mmd`, `mcopy`) instead of `mount`. See the build steps used during initial setup for the exact recipe.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The reference to 'build steps used during initial setup' is vague and not actionable for future contributors who were not part of that process. It is better to describe the approach or provide a generic example of using mtools to avoid the vfat mount requirement.

Suggested change
- The cloud VM kernel does not support `vfat` mounts, so `pf build-package-esp` and `pf test-qemu` fail out-of-the-box. Build the ESP image manually using `mtools` (`mmd`, `mcopy`) instead of `mount`. See the build steps used during initial setup for the exact recipe.
- The cloud VM kernel does not support vfat mounts, so pf build-package-esp and pf test-qemu fail out-of-the-box. In these environments, the ESP image must be created and populated using mtools (e.g., mformat, mmd, mcopy) instead of the mount command.

- `uuid-runtime` (provides `uuidgen`) must be installed for `pf secure-make-auth` to work. Add it alongside the other system packages listed above.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

It is better to maintain a single source of truth for system dependencies. Instead of adding a note here to 'Add it alongside the other system packages', the main list in the 'System dependencies' section (line 25) should be updated to include uuid-runtime. Since line 25 is not in this diff, this note should at least be made more concise.

Suggested change
- `uuid-runtime` (provides `uuidgen`) must be installed for `pf secure-make-auth` to work. Add it alongside the other system packages listed above.
- uuid-runtime (provides uuidgen) is required for pf secure-make-auth to generate unique identifiers for SecureBoot variables.

- The QEMU boot test passes when the `PhoenixGuard` string appears in `out/qemu/serial.log`. Attestation failures (`PG-ATTEST=FAIL`) are expected without SecureBoot enrollment.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This line duplicates the success criteria already mentioned in the 'QEMU testing' section (line 15). To avoid redundancy, this bullet point should focus specifically on the attestation failure caveat.

Suggested change
- The QEMU boot test passes when the `PhoenixGuard` string appears in `out/qemu/serial.log`. Attestation failures (`PG-ATTEST=FAIL`) are expected without SecureBoot enrollment.
- Attestation failures (PG-ATTEST=FAIL) in out/qemu/serial.log are expected when running QEMU tests without prior SecureBoot enrollment.

Loading