From a251474abba2fde5ff9704ed6c3f44353b10107e Mon Sep 17 00:00:00 2001 From: Tim Crawford Date: Fri, 6 Dec 2024 16:32:53 -0700 Subject: [PATCH] Misc cleanups - readme: fix method used for flashing System76 EC - rework Make targets - add missing SPDX IDs - move default target to `.cargo/config.toml` - handle installing toolchain with rustup 1.28.0 - replace win64 with efiapi for ABI - clippy - ignore: `dead_code` - ignore: `clippy::missing_transmute_annotations` - fix: `clippy::unnecessary_get_then_check` Does not address `static_mut_refs`, which will be a hard error in the 2024 edition. --- .cargo/config.toml | 2 ++ .github/workflows/ci.yml | 6 ++-- .gitignore | 7 ++--- Makefile | 61 ++++++++++++++++++++-------------------- README.md | 6 ++-- src/app/bios.rs | 2 +- src/app/mapper.rs | 2 ++ src/app/pci.rs | 2 ++ src/app/sideband.rs | 4 +-- src/text.rs | 38 +++++++++++++------------ 10 files changed, 69 insertions(+), 61 deletions(-) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..85d49dc --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,2 @@ +[build] +target = "x86_64-unknown-uefi" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c5ba4b..cbd6359 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,12 +11,12 @@ jobs: - uses: actions/checkout@v4 - name: Setup Rust toolchain - run: rustup show + run: rustup show active-toolchain || rustup toolchain install - name: clippy env: BASEDIR: "." - run: cargo clippy --target x86_64-unknown-uefi -- -D warnings + run: cargo clippy -- -D warnings continue-on-error: true build: @@ -27,7 +27,7 @@ jobs: - name: Install dependencies run: | sudo apt install --yes make mtools parted - rustup show + rustup show active-toolchain || rustup toolchain install - name: Build UEFI application run: make diff --git a/.gitignore b/.gitignore index d50d40e..8dfb9e0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ -build -firmware -prefix -target +/build +/target +/firmware diff --git a/Makefile b/Makefile index 6df8e29..018ea34 100644 --- a/Makefile +++ b/Makefile @@ -1,30 +1,38 @@ -TARGET?=x86_64-unknown-uefi -export BASEDIR?=system76-firmware-update - -export LD=ld -export RUST_TARGET_PATH=$(CURDIR)/targets -BUILD=build/$(TARGET) - -QEMU?=qemu-system-x86_64 -QEMU_FLAGS=\ - -accel kvm \ - -M q35 \ - -m 1024 \ - -net none \ - -vga std \ - -bios /usr/share/OVMF/OVMF_CODE.fd +# SPDX-License-Identifier: GPL-3.0-only + +TARGET = x86_64-unknown-uefi +UEFI_APP = target/$(TARGET)/release/system76_firmware_update.efi +BUILD = build/$(TARGET) +QEMU = qemu-system-x86_64 +OVMF = /usr/share/OVMF + +export BASEDIR ?= system76-firmware-update + all: $(BUILD)/boot.img +$(UEFI_APP): build + +.PHONY: build +build: + cargo build --release + +.PHONY: clean clean: cargo clean rm -rf build -update: - git submodule update --init --recursive --remote - cargo update - -qemu: $(BUILD)/boot.img - $(QEMU) $(QEMU_FLAGS) $< +.PHONY: qemu +qemu: $(BUILD)/boot.img $(OVMF)/OVMF_VARS.fd $(OVMF)/OVMF_CODE.fd + cp $(OVMF)/OVMF_CODE.fd target/ + cp $(OVMF)/OVMF_VARS.fd target/ + $(QEMU) -enable-kvm -M q35 -m 1024 -vga std \ + -chardev stdio,mux=on,id=debug \ + -device isa-serial,index=2,chardev=debug \ + -device isa-debugcon,iobase=0x402,chardev=debug \ + -drive if=pflash,format=raw,readonly=on,file=target/OVMF_CODE.fd \ + -drive if=pflash,format=raw,readonly=on,file=target/OVMF_VARS.fd \ + -net none \ + $< $(BUILD)/boot.img: $(BUILD)/efi.img dd if=/dev/zero of=$@.tmp bs=512 count=100352 @@ -34,7 +42,8 @@ $(BUILD)/boot.img: $(BUILD)/efi.img dd if=$< of=$@.tmp bs=512 count=98304 seek=2048 conv=notrunc mv $@.tmp $@ -$(BUILD)/efi.img: $(BUILD)/boot.efi res/* +$(BUILD)/efi.img: $(UEFI_APP) res/* + mkdir -p $(@D) dd if=/dev/zero of=$@.tmp bs=512 count=98304 mkfs.vfat $@.tmp mmd -i $@.tmp efi @@ -44,11 +53,3 @@ $(BUILD)/efi.img: $(BUILD)/boot.efi res/* mcopy -i $@.tmp -s res ::$(BASEDIR) if [ -d firmware ]; then mcopy -i $@.tmp -s firmware ::$(BASEDIR); fi mv $@.tmp $@ - -$(BUILD)/boot.efi: Cargo.lock Cargo.toml src/* src/*/* - mkdir -p $(BUILD) - cargo rustc \ - --target $(TARGET) \ - --release \ - -- \ - --emit link=$@ diff --git a/README.md b/README.md index 9095f9d..7864b79 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,6 @@ firmware-update expects the firmware images to have specific names: The mechanism used to apply updates depends on the firmware image: -- coreboot-based SBIOS: firmware-update flashes using [intel-spi](https://github.com/system76/intel-spi) -- System76 EC: firmware-update flashes using [ecflash](https://github.com/system76/ecflash) -- Proprietary: The vendor-provided tools are used +- coreboot-based system firmware: [intel-spi](https://github.com/system76/intel-spi) +- System76 EC: [ectool](https://github.com/system76/ec) +- Proprietary: Vendor-provided tools diff --git a/src/app/bios.rs b/src/app/bios.rs index 7c39496..b1570bc 100644 --- a/src/app/bios.rs +++ b/src/app/bios.rs @@ -407,7 +407,7 @@ impl Component for BiosComponent { area_name ); } - } else if areas.get(area_name).is_some() { + } else if areas.contains_key(area_name) { println!( "{}: found in old firmware, but not found in new firmware", area_name diff --git a/src/app/mapper.rs b/src/app/mapper.rs index 5f12ec7..f1934cd 100644 --- a/src/app/mapper.rs +++ b/src/app/mapper.rs @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: GPL-3.0-only + use intel_spi::{Mapper, PhysicalAddress, VirtualAddress}; pub struct UefiMapper; diff --git a/src/app/pci.rs b/src/app/pci.rs index 2616f79..8eb0bb4 100644 --- a/src/app/pci.rs +++ b/src/app/pci.rs @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: GPL-3.0-only + use core::{mem, slice}; use hwio::{Io, Pio}; use std::prelude::*; diff --git a/src/app/sideband.rs b/src/app/sideband.rs index 18b9c0c..0c809fb 100644 --- a/src/app/sideband.rs +++ b/src/app/sideband.rs @@ -1,6 +1,5 @@ -// Copyright 2018-2021 System76 -// // SPDX-License-Identifier: GPL-3.0-only +// Copyright 2018-2021 System76 use core::ptr; @@ -14,6 +13,7 @@ pub struct Sideband { pub addr: u64, } +#[allow(dead_code)] impl Sideband { pub unsafe fn new(sbreg_phys: usize) -> Self { // On UEFI, physical memory is identity mapped diff --git a/src/text.rs b/src/text.rs index 5344f05..8a64713 100644 --- a/src/text.rs +++ b/src/text.rs @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-only +#![allow(clippy::missing_transmute_annotations)] + use core::ops::Deref; use core::{char, mem}; use orbclient::{Color, Renderer}; @@ -14,15 +16,15 @@ use crate::display::{Display, Output, ScaledDisplay}; #[repr(C)] #[allow(non_snake_case)] pub struct TextDisplay<'a> { - pub Reset: extern "win64" fn(&mut TextDisplay, bool) -> Status, - pub OutputString: extern "win64" fn(&mut TextDisplay, *const u16) -> Status, - pub TestString: extern "win64" fn(&mut TextDisplay, *const u16) -> Status, - pub QueryMode: extern "win64" fn(&mut TextDisplay, usize, &mut usize, &mut usize) -> Status, - pub SetMode: extern "win64" fn(&mut TextDisplay, usize) -> Status, - pub SetAttribute: extern "win64" fn(&mut TextDisplay, usize) -> Status, - pub ClearScreen: extern "win64" fn(&mut TextDisplay) -> Status, - pub SetCursorPosition: extern "win64" fn(&mut TextDisplay, usize, usize) -> Status, - pub EnableCursor: extern "win64" fn(&mut TextDisplay, bool) -> Status, + pub Reset: extern "efiapi" fn(&mut TextDisplay, bool) -> Status, + pub OutputString: extern "efiapi" fn(&mut TextDisplay, *const u16) -> Status, + pub TestString: extern "efiapi" fn(&mut TextDisplay, *const u16) -> Status, + pub QueryMode: extern "efiapi" fn(&mut TextDisplay, usize, &mut usize, &mut usize) -> Status, + pub SetMode: extern "efiapi" fn(&mut TextDisplay, usize) -> Status, + pub SetAttribute: extern "efiapi" fn(&mut TextDisplay, usize) -> Status, + pub ClearScreen: extern "efiapi" fn(&mut TextDisplay) -> Status, + pub SetCursorPosition: extern "efiapi" fn(&mut TextDisplay, usize, usize) -> Status, + pub EnableCursor: extern "efiapi" fn(&mut TextDisplay, bool) -> Status, pub Mode: &'static TextOutputMode, pub mode: Box, @@ -33,22 +35,22 @@ pub struct TextDisplay<'a> { pub display: ScaledDisplay<'a>, } -extern "win64" fn reset(_output: &mut TextDisplay, _extra: bool) -> Status { +extern "efiapi" fn reset(_output: &mut TextDisplay, _extra: bool) -> Status { Status(0) } -extern "win64" fn output_string(output: &mut TextDisplay, string: *const u16) -> Status { +extern "efiapi" fn output_string(output: &mut TextDisplay, string: *const u16) -> Status { unsafe { output.write(string); } Status(0) } -extern "win64" fn test_string(_output: &mut TextDisplay, _string: *const u16) -> Status { +extern "efiapi" fn test_string(_output: &mut TextDisplay, _string: *const u16) -> Status { Status(0) } -extern "win64" fn query_mode( +extern "efiapi" fn query_mode( output: &mut TextDisplay, _mode: usize, columns: &mut usize, @@ -59,21 +61,21 @@ extern "win64" fn query_mode( Status(0) } -extern "win64" fn set_mode(_output: &mut TextDisplay, _mode: usize) -> Status { +extern "efiapi" fn set_mode(_output: &mut TextDisplay, _mode: usize) -> Status { Status(0) } -extern "win64" fn set_attribute(output: &mut TextDisplay, attribute: usize) -> Status { +extern "efiapi" fn set_attribute(output: &mut TextDisplay, attribute: usize) -> Status { output.mode.Attribute = attribute as i32; Status(0) } -extern "win64" fn clear_screen(output: &mut TextDisplay) -> Status { +extern "efiapi" fn clear_screen(output: &mut TextDisplay) -> Status { output.clear(); Status(0) } -extern "win64" fn set_cursor_position( +extern "efiapi" fn set_cursor_position( output: &mut TextDisplay, column: usize, row: usize, @@ -82,7 +84,7 @@ extern "win64" fn set_cursor_position( Status(0) } -extern "win64" fn enable_cursor(output: &mut TextDisplay, enable: bool) -> Status { +extern "efiapi" fn enable_cursor(output: &mut TextDisplay, enable: bool) -> Status { output.mode.CursorVisible = enable; Status(0) }